From: Michael Anthony Knyszek <mknyszek@google.com>
Date: Thu, 16 Nov 2023 17:42:25 +0000 (+0000)
Subject: runtime: put allocation headers back at the start the object
X-Git-Tag: go1.22rc1~267
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0ef169abb1db6bd4e184ee87204dd883fb20cf1c;p=gostls13.git

runtime: put allocation headers back at the start the object

A persistent performance regression was discovered on
perf.golang.org/dashboard and this was narrowed down to the switch to
footers. Using allocation headers instead resolves the issue.

The benchmark results for allocation footers weren't realistic, because
they were performed on a machine with enough L3 cache that it completely
hid the additional cache miss introduced by allocation footers.

This means that in some corner cases the Go runtime may no longer
allocate 16-byte aligned memory. Note however that this property was
*mostly* incidental and never guaranteed in any documentation.

Allocation headers were tested widely within Google and no issues were
found, so we're fairly confident that this will not affect very many
users.

Nonetheless, by Hyrum's Law some code might depend on it. A follow-up
change will add a GODEBUG flag that ensures 16 byte alignment at the
potential cost of some additional memory use. Users experiencing both a
performance regression and an alignment issue can also disable the
GOEXPERIMENT at build time.

This reverts commit 1e250a219900651dad27f29eab0877eee4afd5b9.

Change-Id: Ia7d62a9c60d1773c8b6d33322ee33a80ef814943
Reviewed-on: https://go-review.googlesource.com/c/go/+/543255
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
---

diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go
index 398eaf5d52..c7ab928fe6 100644
--- a/src/runtime/malloc.go
+++ b/src/runtime/malloc.go
@@ -1153,7 +1153,8 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
 				memclrNoHeapPointers(x, size)
 			}
 			if goexperiment.AllocHeaders && hasHeader {
-				header = (**_type)(unsafe.Pointer(uintptr(v) + size - mallocHeaderSize))
+				header = (**_type)(x)
+				x = add(x, mallocHeaderSize)
 				size -= mallocHeaderSize
 			}
 		}
diff --git a/src/runtime/mbitmap_allocheaders.go b/src/runtime/mbitmap_allocheaders.go
index 03cec5ffcc..33535a515a 100644
--- a/src/runtime/mbitmap_allocheaders.go
+++ b/src/runtime/mbitmap_allocheaders.go
@@ -48,9 +48,9 @@
 // is zeroed, so the GC just observes nil pointers.
 // Note that this "tiled" bitmap isn't stored anywhere; it is generated on-the-fly.
 //
-// For objects without their own span, the type metadata is stored in the last
-// word of the allocation slot. For objects with their own span, the type metadata
-// is stored in the mspan.
+// For objects without their own span, the type metadata is stored in the first
+// word before the object at the beginning of the allocation slot. For objects
+// with their own span, the type metadata is stored in the mspan.
 //
 // The bitmap for small unallocated objects in scannable spans is not maintained
 // (can be junk).
@@ -167,7 +167,8 @@ func (span *mspan) typePointersOf(addr, size uintptr) typePointers {
 }
 
 // typePointersOfUnchecked is like typePointersOf, but assumes addr is the base
-// pointer of an object in span. It returns an iterator that generates all pointers
+// of an allocation slot in a span (the start of the object if no header, the
+// header otherwise). It returns an iterator that generates all pointers
 // in the range [addr, addr+span.elemsize).
 //
 // nosplit because it is used during write barriers and must not be preempted.
@@ -192,8 +193,9 @@ func (span *mspan) typePointersOfUnchecked(addr uintptr) typePointers {
 	// All of these objects have a header.
 	var typ *_type
 	if spc.sizeclass() != 0 {
-		// Pull the allocation header from the last word of the object.
-		typ = *(**_type)(unsafe.Pointer(addr + span.elemsize - mallocHeaderSize))
+		// Pull the allocation header from the first word of the object.
+		typ = *(**_type)(unsafe.Pointer(addr))
+		addr += mallocHeaderSize
 	} else {
 		typ = span.largeType
 	}
diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go
index 18cd93e77e..be501e6fca 100644
--- a/src/runtime/mfinal.go
+++ b/src/runtime/mfinal.go
@@ -9,6 +9,7 @@ package runtime
 import (
 	"internal/abi"
 	"internal/goarch"
+	"internal/goexperiment"
 	"runtime/internal/atomic"
 	"runtime/internal/sys"
 	"unsafe"
@@ -410,7 +411,7 @@ func SetFinalizer(obj any, finalizer any) {
 	}
 
 	// find the containing object
-	base, _, _ := findObject(uintptr(e.data), 0, 0)
+	base, span, _ := findObject(uintptr(e.data), 0, 0)
 
 	if base == 0 {
 		if isGoPointerWithoutSpan(e.data) {
@@ -419,6 +420,11 @@ func SetFinalizer(obj any, finalizer any) {
 		throw("runtime.SetFinalizer: pointer not in allocated block")
 	}
 
+	// Move base forward if we've got an allocation header.
+	if goexperiment.AllocHeaders && !span.spanclass.noscan() && !heapBitsInSpan(span.elemsize) && span.spanclass.sizeclass() != 0 {
+		base += mallocHeaderSize
+	}
+
 	if uintptr(e.data) != base {
 		// As an implementation detail we allow to set finalizers for an inner byte
 		// of an object if it could come from tiny alloc (see mallocgc for details).