]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix leak and locking in BenchmarkMSpanCountAlloc
authorMichael Anthony Knyszek <mknyszek@google.com>
Wed, 16 Sep 2020 16:22:28 +0000 (16:22 +0000)
committerMichael Knyszek <mknyszek@google.com>
Wed, 16 Sep 2020 17:07:38 +0000 (17:07 +0000)
CL 249917 made the mspan in MSpanCountAlloc no longer stack-allocated
(for good reason), but then allocated an mspan on each call and did not
free it, resulting in a leak. That allocation was also not protected by
the heap lock, which could lead to data corruption of mheap fields and
the spanalloc.

To fix this, export some functions to allocate/free dummy mspans from
spanalloc (with proper locking) and allocate just one up-front for the
benchmark, freeing it at the end. Then, update MSpanCountAlloc to accept
a dummy mspan.

Note that we need to allocate the dummy mspan up-front otherwise we
measure things like heap locking and fixalloc performance instead of
what we actually want to measure: how fast we can do a popcount on the
mark bits.

Fixes #41391.

Change-Id: If6629a6ec1ece639c7fb78532045837a8c872c04
Reviewed-on: https://go-review.googlesource.com/c/go/+/255297
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>

src/runtime/export_test.go
src/runtime/gc_test.go

index 929bb35db685576f03807d8d5515c068f06d9f7e..e3d6441c18aed04212dad4ed939ef9505bef6452 100644 (file)
@@ -983,9 +983,31 @@ func MapHashCheck(m interface{}, k interface{}) (uintptr, uintptr) {
        return x, y
 }
 
-func MSpanCountAlloc(bits []byte) int {
-       s := (*mspan)(mheap_.spanalloc.alloc())
+// mspan wrapper for testing.
+//go:notinheap
+type MSpan mspan
+
+// Allocate an mspan for testing.
+func AllocMSpan() *MSpan {
+       var s *mspan
+       systemstack(func() {
+               s = (*mspan)(mheap_.spanalloc.alloc())
+       })
+       return (*MSpan)(s)
+}
+
+// Free an allocated mspan.
+func FreeMSpan(s *MSpan) {
+       systemstack(func() {
+               mheap_.spanalloc.free(unsafe.Pointer(s))
+       })
+}
+
+func MSpanCountAlloc(ms *MSpan, bits []byte) int {
+       s := (*mspan)(ms)
        s.nelems = uintptr(len(bits) * 8)
        s.gcmarkBits = (*gcBits)(unsafe.Pointer(&bits[0]))
-       return s.countAlloc()
+       result := s.countAlloc()
+       s.gcmarkBits = nil
+       return result
 }
index c5c8a4cecfba6ec9cdbd6f4bcfd70309e0f0619c..9edebdada68055a5de018ab8732d9ee0ea9e02ea 100644 (file)
@@ -763,6 +763,10 @@ func BenchmarkScanStackNoLocals(b *testing.B) {
 }
 
 func BenchmarkMSpanCountAlloc(b *testing.B) {
+       // Allocate one dummy mspan for the whole benchmark.
+       s := runtime.AllocMSpan()
+       defer runtime.FreeMSpan(s)
+
        // n is the number of bytes to benchmark against.
        // n must always be a multiple of 8, since gcBits is
        // always rounded up 8 bytes.
@@ -774,7 +778,7 @@ func BenchmarkMSpanCountAlloc(b *testing.B) {
 
                        b.ResetTimer()
                        for i := 0; i < b.N; i++ {
-                               runtime.MSpanCountAlloc(bits)
+                               runtime.MSpanCountAlloc(s, bits)
                        }
                })
        }