runtime: rewrite TestPhysicalMemoryUtilization

This test changes TestPhysicalMemoryUtilization to be simpler, more
robust, and more honest about what's going on.

Fixes #49411.

Change-Id: I913ef055c6e166c104c62595c1597d44db62018c
Reviewed-on: https://go-review.googlesource.com/c/go/+/362978
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>

1 files changed, 53 insertions, 68 deletions

diff --git a/src/runtime/testdata/testprog/gc.go b/src/runtime/testdata/testprog/gc.go
index 74732cd9f4..6484c36139 100644
--- a/src/runtime/testdata/testprog/gc.go
+++ b/src/runtime/testdata/testprog/gc.go
@@ -132,81 +132,75 @@ func GCFairness2() {
 func GCPhys() {
 	// This test ensures that heap-growth scavenging is working as intended.
 	//
-	// It sets up a specific scenario: it allocates two pairs of objects whose
-	// sizes sum to size. One object in each pair is "small" (though must be
-	// large enough to be considered a large object by the runtime) and one is
-	// large. The small objects are kept while the large objects are freed,
-	// creating two large unscavenged holes in the heap. The heap goal should
-	// also be small as a result (so size must be at least as large as the
-	// minimum heap size). We then allocate one large object, bigger than both
-	// pairs of objects combined. This allocation, because it will tip
-	// HeapSys-HeapReleased well above the heap goal, should trigger heap-growth
-	// scavenging and scavenge most, if not all, of the large holes we created
-	// earlier.
+	// It attempts to construct a sizeable "swiss cheese" heap, with many
+	// allocChunk-sized holes. Then, it triggers a heap growth by trying to
+	// allocate as much memory as would fit in those holes.
+	//
+	// The heap growth should cause a large number of those holes to be
+	// returned to the OS.
+
 	const (
-		// Size must be also large enough to be considered a large
-		// object (not in any size-segregated span).
-		size    = 4 << 20
-		split   = 64 << 10
-		objects = 2
+		allocTotal = 32 << 20
+		allocChunk = 64 << 10
+		allocs     = allocTotal / allocChunk
 
 		// The page cache could hide 64 8-KiB pages from the scavenger today.
 		maxPageCache = (8 << 10) * 64
-
-		// Reduce GOMAXPROCS down to 4 if it's greater. We need to bound the amount
-		// of memory held in the page cache because the scavenger can't reach it.
-		// The page cache will hold at most maxPageCache of memory per-P, so this
-		// bounds the amount of memory hidden from the scavenger to 4*maxPageCache
-		// at most.
-		maxProcs = 4
 	)
-	// Set GOGC so that this test operates under consistent assumptions.
+	// Set GC percent just so this test is a little more consistent in the
+	// face of varying environments.
 	debug.SetGCPercent(100)
-	procs := runtime.GOMAXPROCS(-1)
-	if procs > maxProcs {
-		defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(maxProcs))
-		procs = runtime.GOMAXPROCS(-1)
-	}
-	// Save objects which we want to survive, and condemn objects which we don't.
-	// Note that we condemn objects in this way and release them all at once in
-	// order to avoid having the GC start freeing up these objects while the loop
-	// is still running and filling in the holes we intend to make.
-	saved := make([][]byte, 0, objects+1)
-	condemned := make([][]byte, 0, objects)
-	for i := 0; i < 2*objects; i++ {
+
+	// Set GOMAXPROCS to 1 to minimize the amount of memory held in the page cache,
+	// and to reduce the chance that the background scavenger gets scheduled.
+	defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(1))
+
+	// Allocate allocTotal bytes of memory in allocChunk byte chunks.
+	// Alternate between whether the chunk will be held live or will be
+	// condemned to GC to create holes in the heap.
+	saved := make([][]byte, allocs/2+1)
+	condemned := make([][]byte, allocs/2)
+	for i := 0; i < allocs; i++ {
+		b := make([]byte, allocChunk)
 		if i%2 == 0 {
-			saved = append(saved, make([]byte, split))
+			saved = append(saved, b)
 		} else {
-			condemned = append(condemned, make([]byte, size-split))
+			condemned = append(condemned, b)
 		}
 	}
-	condemned = nil
-	// Clean up the heap. This will free up every other object created above
-	// (i.e. everything in condemned) creating holes in the heap.
-	// Also, if the condemned objects are still being swept, its possible that
-	// the scavenging that happens as a result of the next allocation won't see
-	// the holes at all. We call runtime.GC() twice here so that when we allocate
-	// our large object there's no race with sweeping.
-	runtime.GC()
+
+	// Run a GC cycle just so we're at a consistent state.
 	runtime.GC()
-	// Perform one big allocation which should also scavenge any holes.
-	//
-	// The heap goal will rise after this object is allocated, so it's very
-	// important that we try to do all the scavenging in a single allocation
-	// that exceeds the heap goal. Otherwise the rising heap goal could foil our
-	// test.
-	saved = append(saved, make([]byte, objects*size))
-	// Clean up the heap again just to put it in a known state.
+
+	// Drop the only reference to all the condemned memory.
+	condemned = nil
+
+	// Clear the condemned memory.
 	runtime.GC()
+
+	// At this point, the background scavenger is likely running
+	// and could pick up the work, so the next line of code doesn't
+	// end up doing anything. That's fine. What's important is that
+	// this test fails somewhat regularly if the runtime doesn't
+	// scavenge on heap growth, and doesn't fail at all otherwise.
+
+	// Make a large allocation that in theory could fit, but won't
+	// because we turned the heap into swiss cheese.
+	saved = append(saved, make([]byte, allocTotal/2))
+
 	// heapBacked is an estimate of the amount of physical memory used by
 	// this test. HeapSys is an estimate of the size of the mapped virtual
 	// address space (which may or may not be backed by physical pages)
 	// whereas HeapReleased is an estimate of the amount of bytes returned
 	// to the OS. Their difference then roughly corresponds to the amount
 	// of virtual address space that is backed by physical pages.
+	//
+	// heapBacked also subtracts out maxPageCache bytes of memory because
+	// this is memory that may be hidden from the scavenger per-P. Since
+	// GOMAXPROCS=1 here, that's fine.
 	var stats runtime.MemStats
 	runtime.ReadMemStats(&stats)
-	heapBacked := stats.HeapSys - stats.HeapReleased
+	heapBacked := stats.HeapSys - stats.HeapReleased - maxPageCache
 	// If heapBacked does not exceed the heap goal by more than retainExtraPercent
 	// then the scavenger is working as expected; the newly-created holes have been
 	// scavenged immediately as part of the allocations which cannot fit in the holes.
@@ -216,19 +210,9 @@ func GCPhys() {
 	// to other allocations that happen during this test we may still see some physical
 	// memory over-use.
 	overuse := (float64(heapBacked) - float64(stats.HeapAlloc)) / float64(stats.HeapAlloc)
-	// Compute the threshold.
-	//
-	// In theory, this threshold should just be zero, but that's not possible in practice.
-	// Firstly, the runtime's page cache can hide up to maxPageCache of free memory from the
-	// scavenger per P. To account for this, we increase the threshold by the ratio between the
-	// total amount the runtime could hide from the scavenger to the amount of memory we expect
-	// to be able to scavenge here, which is (size-split)*objects. This computation is the crux
-	// GOMAXPROCS above; if GOMAXPROCS is too high the threshold just becomes 100%+ since the
-	// amount of memory being allocated is fixed. Then we add 5% to account for noise, such as
-	// other allocations this test may have performed that we don't explicitly account for The
-	// baseline threshold here is around 11% for GOMAXPROCS=1, capping out at around 30% for
-	// GOMAXPROCS=4.
-	threshold := 0.05 + float64(procs)*maxPageCache/float64((size-split)*objects)
+	// Check against our overuse threshold, which is what the scavenger always reserves
+	// to encourage allocation of memory that doesn't need to be faulted in.
+	const threshold = 0.1
 	if overuse <= threshold {
 		fmt.Println("OK")
 		return
@@ -243,6 +227,7 @@ func GCPhys() {
 		"(alloc: %d, goal: %d, sys: %d, rel: %d, objs: %d)\n", threshold*100, overuse*100,
 		stats.HeapAlloc, stats.NextGC, stats.HeapSys, stats.HeapReleased, len(saved))
 	runtime.KeepAlive(saved)
+	runtime.KeepAlive(condemned)
 }
 
 // Test that defer closure is correctly scanned when the stack is scanned.