-
Notifications
You must be signed in to change notification settings - Fork 17.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: better optimization/inlining for sort.Search #43423
Comments
So |
It's a good idea, but has there been any progress on this? I.e., can we move it to the 1.19 milestone? A "modest proposal" for how we might approach this in general -- if a function f takes a function-valued parameter g, bump the threshold for possibly inlining f (more detail -- if g is passed to f and f calls g). If (1) that new threshold puts f into the maybe-inline category and (2) an inlineable g is passed to f at a particular call site, then do the inlining. |
Since there hasn't been any movement since 2020 at this point, I think it's fine to put this in the backlog, personally. |
I'd rather re-milestone to 1.19; the hack I contrived above, might just work, likely to have low impact, and improving sort performance would float a lot of boats. |
@dr2chase Should this to move to Backlog? To 1.20 milestone? |
Bump it to 1.20, and I should pay more attention. Assign it to me, maybe. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
go 1.15.6
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
When I use a helper func
SearchLineInfos()
andSearchFiles
similar assearchInts()
, I test those func benchmarkbenchmark result:
CL: https://go-review.googlesource.com/c/go/+/279447
What did you expect to see?
In
position.go
, we impl helper func for sort.Search.What did you see instead?
Remove
func searchInts(a []int, x int) int
cc @mdempsky
The text was updated successfully, but these errors were encountered: