Skip to content

Commit

Permalink
strings: prevent copyCheck from forcing Builder to escape and allocate
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
bradfitz committed Jan 9, 2018
1 parent b26c88d commit 484586c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
19 changes: 18 additions & 1 deletion src/strings/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,26 @@ type Builder struct {
buf []byte
}

// noescape hides a pointer from escape analysis. noescape is
// the identity function but escape analysis doesn't think the
// output depends on the input. noescape is inlined and currently
// compiles down to zero instructions.
// USE CAREFULLY!
// This was copied from the runtime; see issues 23382 and 7921.
//go:nosplit
func noescape(p unsafe.Pointer) unsafe.Pointer {
x := uintptr(p)
return unsafe.Pointer(x ^ 0)
}

func (b *Builder) copyCheck() {
if b.addr == nil {
b.addr = b
// This hack works around a failing of Go's escape analysis
// that was causing b to escape and be heap allocated.
// See issue 23382.
// TODO: once issue 7921 is fixed, this should be reverted to
// just "b.addr = b".
b.addr = (*Builder)(noescape(unsafe.Pointer(b)))
} else if b.addr != b {
panic("strings: illegal use of non-zero Builder copied by value")
}
Expand Down
12 changes: 12 additions & 0 deletions src/strings/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,18 @@ func TestBuilderAllocs(t *testing.T) {
if allocs > 0 {
t.Fatalf("got %d alloc(s); want 0", allocs)
}

// Issue 23382; verify that copyCheck doesn't force the
// Builder to escape and be heap allocated.
n := testing.AllocsPerRun(10000, func() {
var b Builder
b.Grow(5)
b.WriteString("abcde")
_ = b.String()
})
if n != 1 {
t.Errorf("Builder allocs = %v; want 1", n)
}
}

func numAllocs(fn func()) uint64 {
Expand Down

0 comments on commit 484586c

Please sign in to comment.