runtime: make explicit nil check in heapSetTypeSmallHeader

This is another case very similar to CL 684015 and #74375.

In spans with type headers, mallocgc always writes to the page before
returning the allocated memory. This initial write is done by
runtime.heapSetTypeSmallHeader. Prior to the write, the compiler inserts
a nil check, implemented as a dummy instruction reading from memory.

On a freshly mapped page, this read triggers a page fault, mapping the
zero page read-only. Immediately afterwards, the write triggers another
page fault, copying to a writeable page and performing a TLB flush.

This problem is exacerbated as the process scales up. At GOMAXPROCS=6,
the tile38 sweet benchmark spends around 0.1% of cycles directly
handling these page faults. On the same machine at GOMAXPROCS=192, it
spends about 2.7% of cycles directly handling these page faults.

Replacing the read with an explicit nil check reduces the direct cost of
these page faults down to around 0.1% at GOMAXPROCS=192. There are
additional positive side-effects due to reduced contention, so the
overall time spent in page faults drops from around 12.8% to 6.8%. Most
of the remaining time in page faults is spent on automatic NUMA page
migration (completely unrelated to this issue).

Impact on the tile38 benchmark results:

                    │  baseline   │             cl704755              │
                    │   sec/op    │   sec/op     vs base              │
Tile38QueryLoad-192   1.638m ± 3%   1.494m ± 5%  -8.79% (p=0.002 n=6)

                    │     baseline      │              cl704755              │
                    │ average-RSS-bytes │ average-RSS-bytes  vs base         │
Tile38QueryLoad-192        5.384Gi ± 3%        5.399Gi ± 3%  ~ (p=0.818 n=6)

                    │    baseline    │            cl704755             │
                    │ peak-RSS-bytes │ peak-RSS-bytes  vs base         │
Tile38QueryLoad-192     5.818Gi ± 1%     5.864Gi ± 2%  ~ (p=0.394 n=6)

                    │   baseline    │            cl704755            │
                    │ peak-VM-bytes │ peak-VM-bytes  vs base         │
Tile38QueryLoad-192    7.121Gi ± 1%    7.180Gi ± 2%  ~ (p=0.818 n=6)

                    │    baseline     │               cl704755                │
                    │ p50-latency-sec │ p50-latency-sec  vs base              │
Tile38QueryLoad-192       343.2µ ± 1%       313.2µ ± 3%  -8.73% (p=0.002 n=6)

                    │    baseline     │             cl704755             │
                    │ p90-latency-sec │ p90-latency-sec  vs base         │
Tile38QueryLoad-192       1.662m ± 2%       1.603m ± 5%  ~ (p=0.093 n=6)

                    │    baseline     │                cl704755                │
                    │ p99-latency-sec │ p99-latency-sec  vs base               │
Tile38QueryLoad-192       41.56m ± 8%      35.26m ± 18%  -15.17% (p=0.026 n=6)

                    │  baseline   │             cl704755              │
                    │    ops/s    │    ops/s     vs base              │
Tile38QueryLoad-192   87.89k ± 3%   96.36k ± 4%  +9.64% (p=0.002 n=6)

Updates #74375.

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

diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index 9872e5297fb4b7618f6c9e7bf26e8c711a8c0be8..508de9a115723c1b17a5ae11f5b3518d129b6969 100644 (file)
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -714,6 +714,26 @@ func heapSetTypeNoHeader(x, dataSize uintptr, typ *_type, span *mspan) uintptr {
 }
 
 func heapSetTypeSmallHeader(x, dataSize uintptr, typ *_type, header **_type, span *mspan) uintptr {
+       if header == nil {
+               // This nil check and throw is almost pointless. Normally we would
+               // expect header to never be nil. However, this is called on potentially
+               // freshly-allocated virtual memory. As of 2025, the compiler-inserted
+               // nil check is not a branch but a memory read that we expect to fault
+               // if the pointer really is nil.
+               //
+               // However, this causes a read of the page, and operating systems may
+               // take it as a hint to back the accessed memory with a read-only zero
+               // page. However, we immediately write to this memory, which can then
+               // force operating systems to have to update the page table and flush
+               // the TLB.
+               //
+               // This nil check is thus an explicit branch instead of what the compiler
+               // would insert circa 2025, which is a memory read instruction.
+               //
+               // See go.dev/issue/74375 for details of a similar issue in
+               // spanInlineMarkBits.
+               throw("runtime: pointer to heap type header nil?")
+       }
        *header = typ
        if doubleCheckHeapSetType {
                doubleCheckHeapType(x, dataSize, typ, header, span)