Skip to content

feature: improve performance if the element of the Value of *Model does not implement the AfterFindable interface. #799

New issue
New issue
Closed
#805
Closed
feature: improve performance if the element of the Value of *Model does not implement the AfterFindable interface.#799
#805
Assignees
sio4
Labels
s: acceptedThis proposal was accepted. Someone can start working on it.
Milestone
 
v6.1.1
@seedeed

Description

@seedeed
seedeed
opened on Nov 26, 2022

Description

I found that the afterFind method of *Model would create a log of goroutines via *errgroup.Group.Go(...) even if the element of the Value of *Model does not implement the AfterFindable interface, which might cause a lot of unneeded CPU cost. To avoid this, I think we could move the AfterFindable interface checking out of the Go function. If this's really a problem, I think I could submit a PR to fix it.

pop/callbacks.go

Lines 32 to 43 in 17f09c0

for i := 0; i < rv.Len(); i++ {
func(i int) {
wg.Go(func() error {
y := rv.Index(i)
y = y.Addr()
if x, ok := y.Interface().(AfterFindable); ok {
return x.AfterFind(c)
}
return nil
})
}(i)
}

Additional Information

No response

Activity

github-actions

github-actions commented on Dec 27, 2022

@github-actions
github-actions
on Dec 27, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

github-actions
added
stale
on Dec 27, 2022
github-actions

github-actions commented on Jan 3, 2023

@github-actions
github-actions
on Jan 3, 2023

This issue was closed because it has been stalled for 30+7 days with no activity.

github-actions
added
s: closed
on Jan 3, 2023
github-actions
closed this as not plannedon Jan 3, 2023
sio4
added
s: triageSome tests need to be run to confirm the issue
and removed
stale
s: closed
on Jan 6, 2023
sio4
reopened this on Jan 6, 2023
sio4
added a commit that references this issue on Jan 13, 2023

limit invoking go routines when only the model is AfterFindable (fixes …

89774fa
sio4
mentioned this on Jan 13, 2023
sio4

sio4 commented on Jan 13, 2023

@sio4
sio4
on Jan 13, 2023
Member

Hi @seedeed, Thank you for reporting this issue. I agree with you, the block could spend unnecessary resources.

The block was recently touched with a new thing, and I opened a PR for your issue after that. The PR is #805, and the specific commit is 89774fa. Please take a look at the PR and/or the commit and give me feedback if the direction is the same as your idea, or please let me know your idea if you have another good way.

Thanks in advance!

sio4
added this to the v6.1.1 milestone on Jan 13, 2023
sio4
self-assigned this
on Jan 13, 2023
sio4
removed
s: triageSome tests need to be run to confirm the issue
on Jan 13, 2023

4 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

s: acceptedThis proposal was accepted. Someone can start working on it.

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions
    feature: improve performance if the element of the Value of *Model does not implement the AfterFindable interface. · Issue #799 · gobuffalo/pop