-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: inlining callback functions #70260
Comments
Not a proposal https://github.com/golang/proposal#readme. Taking out of the proposal process. |
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
I think this is the same root cause as #69411 ; there's a glitch in the "constant propagation" for some-but-not-all closures. |
If you could include that benchmark here, or put it up on the playground (won't run as a benchmark but that is okay with me) then I could add it to the collection of "is this really all the same problem?" bugs. |
func SeparateIntoTwo[T any](slice []T, isFirstGroupFunc func(T) bool) [2][]T {
if len(slice) == 0 {
return [2][]T{}
}
result := [2][]T{
make([]T, 0, len(slice)/2),
make([]T, 0, len(slice)/2),
}
for _, elem := range slice {
if isFirstGroupFunc(elem) {
result[0] = append(result[0], elem)
} else {
result[1] = append(result[1], elem)
}
}
return result
}
type Product struct {
Id int
IsDeleted bool
}
func (p Product) GetIsDeleted() bool {
return p.IsDeleted
}
func SeparateIntoTwo_WithoutCallback(slice []Product) [2][]Product {
if len(slice) == 0 {
return [2][]Product{}
}
result := [2][]Product{
make([]Product, 0, len(slice)/2),
make([]Product, 0, len(slice)/2),
}
for _, elem := range slice {
if elem.GetIsDeleted() {
result[0] = append(result[0], elem)
} else {
result[1] = append(result[1], elem)
}
}
return result
} // goos: darwin
// goarch: arm64
// cpu: Apple M1 Pro
// Benchmark_SeparateIntoTwo
// Benchmark_SeparateIntoTwo/callback:_1E+02_elements-8 2308760 523.1 ns/op 2560 B/op 2 allocs/op
// Benchmark_SeparateIntoTwo/method:_1E+02_elements-8 2880152 415.3 ns/op 2560 B/op 2 allocs/op
// Benchmark_SeparateIntoTwo/callback:_1E+04_elements-8 33636 35649 ns/op 245760 B/op 2 allocs/op
// Benchmark_SeparateIntoTwo/method:_1E+04_elements-8 41936 29857 ns/op 245760 B/op 2 allocs/op
// Benchmark_SeparateIntoTwo/callback:_1E+06_elements-8 415 2879534 ns/op 24002566 B/op 2 allocs/op
// Benchmark_SeparateIntoTwo/method:_1E+06_elements-8 562 2103189 ns/op 24002565 B/op 2 allocs/op
func Benchmark_SeparateIntoTwo(b *testing.B) {
for itemsCount := 100; itemsCount <= 1e6; itemsCount *= 100 {
slice := make([]Product, 0, itemsCount)
for i := 0; i < itemsCount; i++ {
slice = append(slice, Product{Id: i, IsDeleted: i%2 == 0})
}
b.Run(fmt.Sprintf("callback: %.0E elements", float64(itemsCount)), func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = SeparateIntoTwo(slice, Product.GetIsDeleted)
}
})
b.Run(fmt.Sprintf("method: %.0E elements", float64(itemsCount)), func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = SeparateIntoTwo_WithoutCallback(slice)
}
})
}
} |
This looks fixed on tip (and faster). Thanks for the reproducer, that was helpful. |
Proposal Details
I'm sorry if this is a duplicate. I honestly tried to find a similar issue
go 1.23
MacOS
I wrote simple generic functions to simplify the work.
Example:
Most of them accept callback functions for various groupings and aggregations.
I compared the performance with the same function, but instead of a callback, it used a method.
I found that the performance of functions with callback is slightly lower than without them.
The reason is that the methods were inline by the compiler, while the callback remained unchanged.
Moreover, the callback is not inline even when the
SeparateIntoTwo
function itself is inline. At this point, the compiler had information about the complexity of the function being passed as an argument.An attempt to inline a callback function by the compiler in cases where the function taking the callback as an argument is also inline
Lack of inlining
The text was updated successfully, but these errors were encountered: