Skip to content

Commit 484586c

Browse files
committed
strings: prevent copyCheck from forcing Builder to escape and allocate
All credit and blame goes to Ian for this suggestion, copied from the runtime. Fixes #23382 Updates #7921 Change-Id: I3d5a9ee4ab730c87e0f3feff3e7fceff9bcf9e18 Reviewed-on: https://go-review.googlesource.com/86976 Reviewed-by: Ian Lance Taylor <iant@golang.org>
1 parent b26c88d commit 484586c

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

src/strings/builder.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,26 @@ type Builder struct {
1717
buf []byte
1818
}
1919

20+
// noescape hides a pointer from escape analysis. noescape is
21+
// the identity function but escape analysis doesn't think the
22+
// output depends on the input. noescape is inlined and currently
23+
// compiles down to zero instructions.
24+
// USE CAREFULLY!
25+
// This was copied from the runtime; see issues 23382 and 7921.
26+
//go:nosplit
27+
func noescape(p unsafe.Pointer) unsafe.Pointer {
28+
x := uintptr(p)
29+
return unsafe.Pointer(x ^ 0)
30+
}
31+
2032
func (b *Builder) copyCheck() {
2133
if b.addr == nil {
22-
b.addr = b
34+
// This hack works around a failing of Go's escape analysis
35+
// that was causing b to escape and be heap allocated.
36+
// See issue 23382.
37+
// TODO: once issue 7921 is fixed, this should be reverted to
38+
// just "b.addr = b".
39+
b.addr = (*Builder)(noescape(unsafe.Pointer(b)))
2340
} else if b.addr != b {
2441
panic("strings: illegal use of non-zero Builder copied by value")
2542
}

src/strings/builder_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,18 @@ func TestBuilderAllocs(t *testing.T) {
180180
if allocs > 0 {
181181
t.Fatalf("got %d alloc(s); want 0", allocs)
182182
}
183+
184+
// Issue 23382; verify that copyCheck doesn't force the
185+
// Builder to escape and be heap allocated.
186+
n := testing.AllocsPerRun(10000, func() {
187+
var b Builder
188+
b.Grow(5)
189+
b.WriteString("abcde")
190+
_ = b.String()
191+
})
192+
if n != 1 {
193+
t.Errorf("Builder allocs = %v; want 1", n)
194+
}
183195
}
184196

185197
func numAllocs(fn func()) uint64 {

0 commit comments

Comments
 (0)