-
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: stack allocate string and slice headers when passed through non-escaping interfaces #23676
Comments
The escape analysis does not track whether things are modified. So it cannot make an interface with its data field directly pointing to s. That said, it is possible to have it point to a copy of s on stack. However, currently the escape analysis doesn't distinguish the interface and its underlying value, i.e. |
If the caller knew that that the last use of the string was the function call itself, then it wouldn't even need to make a copy on the stack. |
Once we fix the escape problem, this will probably not matter. |
Isn't this issue a duplicate of #8618 ? |
It looks this is a problem of inlining: package main
import "testing"
//go:noinline
func NewA(x ...interface{}) string {
if len(x) == 0 {
return ""
}
return x[0].(string)
}
//go:noinline
func NewB(x []interface{}) string {
if len(x) == 0 {
return ""
}
return x[0].(string)
}
//go:noinline
func NewC(x ...interface{}) string {
return NewB(x)
}
var sink string
func BenchmarkA(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s = "hello"
sink = NewA(s)
}
}
func BenchmarkB(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s = "hello"
sink = NewB([]interface{}{s})
}
}
func BenchmarkC(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s = "hello"
sink = NewC(s)
}
} Output:
After removing the
|
And this is not a string specified problem:
The output:
|
The smallest case: package main
import "testing"
func NewA(x ...interface{}) int {
return 0
}
var sink int
func BenchmarkA(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s int = 99999
sink = NewA(s)
}
}
func BenchmarkB(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
var s int = 99999
sink = NewA([]interface{}{s}...)
}
} It looks the code inlining misleads the escape analysis, so that the arguments in the auto-constructed slice parameter are all allocated on heap. {update]: if the value |
It seems a optimization introduced by CL 216401. |
It looks this has been fixed on tip. |
@dsnet this can be closed, right? |
Correct. Confirmed that it is fixed. |
Consider the following snippet:
Running benchmarks on this, you find:
Version A is more costly by 1 extra allocation. This occurs because calling
NewA
has a hidden call toruntime.convT2Estring
where the string header is allocated on the heap.However, I contend that the compiler should be able to prove that this is unnecessary. The call to
NewA
is concrete, and so the compiler should able to prove that a string header passed into the variadic interfaces neither escaped nor is modified. When crafting the interface header, it should be able to directly point to the string header on the stack.\cc @randall77 @neild
The text was updated successfully, but these errors were encountered: