golang / golang/go

cmd/compile: performance regression with returned structs in 1.27.0 due to CL 748200

Open
#80,980 3 comments 0 reactions 1 assignee Claimed by @JunyangShao View on GitHub
compiler/runtime NeedsInvestigation
Dominant language
Go
Stars
139k
Forks
19.4k
PR merge metrics
PR metrics pending

Description

First of all, thank you for all your hard work on go 1.27.0.

On upgrading from 1.26.4 to 1.27.0 I noticed a major performance regression on one of my benchmarks. This is a simple reproducer:

```go
type T struct {
s []byte
x, y int
}

func newT(s []byte) T { return T{s: s} }

func Append(s []byte) []byte {
t := newT(s)
t.s = append(t.s, 1)
return t.s
}
```

In 1.27.0 the function is 47% slower (linux/amd64). What's happening is that when you have a value-returning constructor whose result is assigned to a local and then mutated the struct gets built twice on the stack instead of once. Bisecting reveals that [CL 748200](https://go.dev/cl/748200) is responsible. Amusingly this was supposed to fix #77720, a very similar problem. The fix in 1.27.0 does fix that case, but it also causes broader performance regressions.

I believe the fix is as simple as the following:

```diff
--- a/src/cmd/compile/internal/ssa/_gen/generic.rules
+++ b/src/cmd/compile/internal/ssa/_gen/generic.rules
@@ -823,7 +823,7 @@
// Load from a region just copied by Move can read directly from the source.
(Load op:(OffPtr [o1] p1) move:(Move [n] p2 src mem))
&& o1 >= 0 && o1+t1.Size() <= n && isSamePtr(p1, p2)
- && !isVolatile(src)
+ && !isVolatile(src) && !isStackPtr(src)
=> @move.Block (Load (OffPtr [o1] src) mem)
```

This patch results in identical codegen for my example as 1.26.4 whilst still ensuring that the original issue remains fixed. I'd be happy to raise a PR if that would be useful.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.