runtime: add more precise test of assist credit handling for runtime.freegc

This CL is part of a set of CLs that attempt to reduce how much work the
GC must do. See the design in https://go.dev/design/74299-runtime-freegc

This CL adds a better test of assist credit handling when heap objects
are being reused after a runtime.freegc call.

The main approach is bracketing alloc/free pairs with measurements
of the assist credit before and after, and hoping to see a net zero
change in the assist credit.

However, validating the desired behavior is perhaps a bit subtle.
To help stabilize the measurements, we do acquirem in the test code
to avoid being preempted during the measurements to reduce other code's
ability to adjust the assist credit while we are measuring, and
we also reduce GOMAXPROCS to 1.

This test currently does fail if we deliberately introduce bugs
in the runtime.freegc implementation such as if we:
- never adjust the assist credit when reusing an object, or
- always adjust the assist credit when reusing an object, or
- deliberately mishandle internal fragmentation.

The two main cases of current interest for testing runtime.freegc
are when over the course of our bracketed measurements gcBlackenEnable
is either true or false. The test attempts to exercise both of those
case by running the GC continually in the background (which we can see
seems effective based on logging and by how our deliberate bugs fail).

This passes ~10K test executions locally via stress.

A small note to the future: a previous incarnation of this test (circa
patchset 11 of this CL) did not do acquirem but had an approach of
ignoring certain measurements, which also was able to pass ~10K runs
via stress. The current version in this CL is simpler, but
recording the existence of the prior version here in case it is
useful in the future. (Hopefully not.)

Updates #74299

Change-Id: I46c7e0295d125f5884fee0cc3d3d31aedc7e5ff4
Reviewed-on: https://go-review.googlesource.com/c/go/+/717520
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Junyang Shao <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: thepudds

src/runtime/export_test.go

@@ -644,6 +644,25 @@ func Freegc(p unsafe.Pointer, size uintptr, noscan bool) {
 	freegc(p, size, noscan)
 }
 
+// Expose gcAssistBytes for the current g for testing.
+func AssistCredit() int64 {
+	assistG := getg()
+	if assistG.m.curg != nil {
+		assistG = assistG.m.curg
+	}
+	return assistG.gcAssistBytes
+}
+
+// Expose gcBlackenEnabled for testing.
+func GcBlackenEnable() bool {
+	// Note we do a non-atomic load here.
+	// Some checks against gcBlackenEnabled (e.g., in mallocgc)
+	// are currently done via non-atomic load for performance reasons,
+	// but other checks are done via atomic load (e.g., in mgcmark.go),
+	// so interpreting this value in a test may be subtle.
+	return gcBlackenEnabled != 0
+}
+
 const SizeSpecializedMallocEnabled = sizeSpecializedMallocEnabled
 
 const RuntimeFreegcEnabled = runtimeFreegcEnabled
@@ -1487,6 +1506,15 @@ func Releasem() {
 	releasem(getg().m)
 }
 
+// GoschedIfBusy is an explicit preemption check to call back
+// into the scheduler. This is useful for tests that run code
+// which spend most of their time as non-preemptible, as it
+// can be placed right after becoming preemptible again to ensure
+// that the scheduler gets a chance to preempt the goroutine.
+func GoschedIfBusy() {
+	goschedIfBusy()
+}
+
 type PIController struct {
 	piController
 }

src/runtime/malloc_test.go

@@ -249,6 +249,7 @@ func TestFreegc(t *testing.T) {
 		{"size=500", testFreegc[[500]byte], true},
 		{"size=512", testFreegc[[512]byte], true},
 		{"size=4096", testFreegc[[4096]byte], true},
+		{"size=20000", testFreegc[[20000]byte], true},       // not power of 2 or spc boundary
 		{"size=32KiB-8", testFreegc[[1<<15 - 8]byte], true}, // max noscan small object for 64-bit
 	}
 
@@ -300,7 +301,7 @@ func testFreegc[T comparable](noscan bool) func(*testing.T) {
 			t.Helper()
 			var zero T
 			if *p != zero {
-				t.Fatalf("found non-zero memory before freeing (tests do not modify memory): %v", *p)
+				t.Fatalf("found non-zero memory before freegc (tests do not modify memory): %v", *p)
 			}
 			runtime.Freegc(unsafe.Pointer(p), unsafe.Sizeof(*p), noscan)
 		}
@@ -405,7 +406,7 @@ func testFreegc[T comparable](noscan bool) func(*testing.T) {
 			// Confirm we are graceful if we have more freed elements at once
 			// than the max free list size.
 			s := make([]*T, 0, 1000)
-			iterations := stressMultiple * stressMultiple // currently 1 or 100 depending on -short
+			iterations := stressMultiple * stressMultiple // currently 1 (-short) or 100
 			for range iterations {
 				s = s[:0]
 				for range 1000 {
@@ -431,7 +432,7 @@ func testFreegc[T comparable](noscan bool) func(*testing.T) {
 					p := alloc()
 					uptr := uintptr(unsafe.Pointer(p))
 					if live[uptr] {
-						t.Fatalf("TestFreeLive: found duplicate pointer (0x%x). i: %d j: %d", uptr, i, j)
+						t.Fatalf("found duplicate pointer (0x%x). i: %d j: %d", uptr, i, j)
 					}
 					live[uptr] = true
 					s = append(s, p)
@@ -451,7 +452,7 @@ func testFreegc[T comparable](noscan bool) func(*testing.T) {
 			// Use explicit free, but the free happens on a different goroutine than the alloc.
 			// This also lightly simulates how the free code sees P migration or flushing
 			// the mcache, assuming we have > 1 P. (Not using testing.AllocsPerRun here).
-			iterations := 10 * stressMultiple * stressMultiple // currently 10 or 1000 depending on -short
+			iterations := 10 * stressMultiple * stressMultiple // currently 10 (-short) or 1000
 			for _, capacity := range []int{2} {
 				for range iterations {
 					ch := make(chan *T, capacity)
@@ -501,6 +502,90 @@ func testFreegc[T comparable](noscan bool) func(*testing.T) {
 				wg.Wait()
 			}
 		})
+
+		t.Run("assist-credit", func(t *testing.T) {
+			// Allocate and free using the same span class repeatedly while
+			// verifying it results in a net zero change in assist credit.
+			// This helps double-check our manipulation of the assist credit
+			// during mallocgc/freegc, including in cases when there is
+			// internal fragmentation when the requested mallocgc size is
+			// smaller than the size class.
+			//
+			// See https://go.dev/cl/717520 for some additional discussion,
+			// including how we can deliberately cause the test to fail currently
+			// if we purposefully introduce some assist credit bugs.
+			if SizeSpecializedMallocEnabled {
+				// TODO(thepudds): skip this test at this point in the stack; later CL has
+				// integration with sizespecializedmalloc.
+				t.Skip("temporarily skip assist credit test for GOEXPERIMENT=sizespecializedmalloc")
+			}
+			if !RuntimeFreegcEnabled {
+				t.Skip("skipping assist credit test with runtime.freegc disabled")
+			}
+
+			// Use a background goroutine to continuously run the GC.
+			done := make(chan struct{})
+			defer close(done)
+			go func() {
+				for {
+					select {
+					case <-done:
+						return
+					default:
+						runtime.GC()
+					}
+				}
+			}()
+
+			// If making changes related to this test, consider testing locally with
+			// larger counts, like 100K or 1M.
+			counts := []int{1, 2, 10, 100 * stressMultiple}
+			// Dropping down to GOMAXPROCS=1 might help reduce noise.
+			defer GOMAXPROCS(GOMAXPROCS(1))
+			size := int64(unsafe.Sizeof(*new(T)))
+			for _, count := range counts {
+				// Start by forcing a GC to reset this g's assist credit
+				// and perhaps help us get a cleaner measurement of GC cycle count.
+				runtime.GC()
+				for i := range count {
+					// We disable preemption to reduce other code's ability to adjust this g's
+					// assist credit or otherwise change things while we are measuring.
+					Acquirem()
+
+					// We do two allocations per loop, with the second allocation being
+					// the one we measure. The first allocation tries to ensure at least one
+					// reusable object on the mspan's free list when we do our measured allocation.
+					p := alloc()
+					free(p)
+
+					// Now do our primary allocation of interest, bracketed by measurements.
+					// We measure more than we strictly need (to log details in case of a failure).
+					creditStart := AssistCredit()
+					blackenStart := GcBlackenEnable()
+					p = alloc()
+					blackenAfterAlloc := GcBlackenEnable()
+					creditAfterAlloc := AssistCredit()
+					free(p)
+					blackenEnd := GcBlackenEnable()
+					creditEnd := AssistCredit()
+
+					Releasem()
+					GoschedIfBusy()
+
+					delta := creditEnd - creditStart
+					if delta != 0 {
+						t.Logf("assist credit non-zero delta: %d", delta)
+						t.Logf("\t| size: %d i: %d count: %d", size, i, count)
+						t.Logf("\t| credit before: %d credit after: %d", creditStart, creditEnd)
+						t.Logf("\t| alloc delta: %d free delta: %d",
+							creditAfterAlloc-creditStart, creditEnd-creditAfterAlloc)
+						t.Logf("\t| gcBlackenEnable (start / after alloc / end): %v/%v/%v",
+							blackenStart, blackenAfterAlloc, blackenEnd)
+						t.FailNow()
+					}
+				}
+			}
+		})
 	}
 }