runtime: rewrite pacer max trigger calculation

Currently the maximum trigger calculation is totally incorrect with
respect to the comment above it and its intent. This change rectifies
this mistake.

For #48409.

Change-Id: Ifef647040a8bdd304dd327695f5f315796a61a74
Reviewed-on: https://go-review.googlesource.com/c/go/+/398834
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 0e64b87317f66dc623e34c198f845db5827d13ae..6d1cf645d207ad91fa22d07698bb3dd9f03f0399 100644 (file)
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -1262,6 +1262,7 @@ const Raceenabled = raceenabled
 const (
        GCBackgroundUtilization = gcBackgroundUtilization
        GCGoalUtilization       = gcGoalUtilization
+       DefaultHeapMinimum      = defaultHeapMinimum
 )
 
 type GCController struct {

diff --git a/src/runtime/mgcpacer.go b/src/runtime/mgcpacer.go
index 57c2215b4f57c60ae6e9ef80a08e088ca47b8cef..40cf67c747e3372f27d8dc17cbc24c860644866b 100644 (file)
--- a/src/runtime/mgcpacer.go
+++ b/src/runtime/mgcpacer.go
@@ -984,19 +984,19 @@ func (c *gcControllerState) commit() {
                minTrigger = triggerBound
        }
 
-       // For small heaps, set the max trigger point at 95% of the heap goal.
-       // This ensures we always have *some* headroom when the GC actually starts.
-       // For larger heaps, set the max trigger point at the goal, minus the
-       // minimum heap size.
+       // For small heaps, set the max trigger point at 95% of the way from the
+       // live heap to the heap goal. This ensures we always have *some* headroom
+       // when the GC actually starts. For larger heaps, set the max trigger point
+       // at the goal, minus the minimum heap size.
+       //
        // This choice follows from the fact that the minimum heap size is chosen
        // to reflect the costs of a GC with no work to do. With a large heap but
        // very little scan work to perform, this gives us exactly as much runway
        // as we would need, in the worst case.
-       maxRunway := uint64(0.95 * float64(goal-c.heapMarked))
-       if largeHeapMaxRunway := goal - c.heapMinimum; goal > c.heapMinimum && maxRunway < largeHeapMaxRunway {
-               maxRunway = largeHeapMaxRunway
+       maxTrigger := uint64(0.95*float64(goal-c.heapMarked)) + c.heapMarked
+       if goal > defaultHeapMinimum && goal-defaultHeapMinimum > maxTrigger {
+               maxTrigger = goal - defaultHeapMinimum
        }
-       maxTrigger := maxRunway + c.heapMarked
        if maxTrigger < minTrigger {
                maxTrigger = minTrigger
        }

diff --git a/src/runtime/mgcpacer_test.go b/src/runtime/mgcpacer_test.go
index 23628898d481b72758ffe7040005fbc464fb07d9..fa60ddcc595efda6af86515cf1f4a1fbd829bee3 100644 (file)
--- a/src/runtime/mgcpacer_test.go
+++ b/src/runtime/mgcpacer_test.go
@@ -34,8 +34,7 @@ func TestGcPacer(t *testing.T) {
                        checker: func(t *testing.T, c []gcCycleResult) {
                                n := len(c)
                                if n >= 25 {
-                                       // For the pacer redesign, assert something even stronger: at this alloc/scan rate,
-                                       // it should be extremely close to the goal utilization.
+                                       // At this alloc/scan rate, the pacer should be extremely close to the goal utilization.
                                        assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, GCGoalUtilization, 0.005)
 
                                        // Make sure the pacer settles into a non-degenerate state in at least 25 GC cycles.
@@ -280,6 +279,114 @@ func TestGcPacer(t *testing.T) {
                                }
                        },
                },
+               {
+                       // This test sets a slow allocation rate and a small heap (close to the minimum heap size)
+                       // to try to minimize the difference between the trigger and the goal.
+                       name:          "SmallHeapSlowAlloc",
+                       gcPercent:     100,
+                       globalsBytes:  32 << 10,
+                       nCores:        8,
+                       allocRate:     constant(1.0),
+                       scanRate:      constant(2048.0),
+                       growthRate:    constant(2.0).sum(ramp(-1.0, 3)),
+                       scannableFrac: constant(0.01),
+                       stackBytes:    constant(8192),
+                       length:        50,
+                       checker: func(t *testing.T, c []gcCycleResult) {
+                               n := len(c)
+                               if n > 4 {
+                                       // After the 4th GC, the heap will stop growing.
+                                       // First, let's make sure we're finishing near the goal, with some extra
+                                       // room because we're probably going to be triggering early.
+                                       assertInRange(t, "goal ratio", c[n-1].goalRatio(), 0.925, 1.025)
+                                       // Next, let's make sure there's some minimum distance between the goal
+                                       // and the trigger.
+                                       //
+                                       // TODO(mknyszek): This is not quite intentional. For small heaps we should
+                                       // be within 5%.
+                                       assertInRange(t, "runway", c[n-1].runway(), 32<<10, 64<<10)
+                               }
+                               if n > 25 {
+                                       // Double-check that GC utilization looks OK.
+
+                                       // At this alloc/scan rate, the pacer should be extremely close to the goal utilization.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, GCGoalUtilization, 0.005)
+                                       // Make sure GC utilization has mostly levelled off.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[n-2].gcUtilization, 0.05)
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[11].gcUtilization, 0.05)
+                               }
+                       },
+               },
+               {
+                       // This test sets a slow allocation rate and a medium heap (around 10x the min heap size)
+                       // to try to minimize the difference between the trigger and the goal.
+                       name:          "MediumHeapSlowAlloc",
+                       gcPercent:     100,
+                       globalsBytes:  32 << 10,
+                       nCores:        8,
+                       allocRate:     constant(1.0),
+                       scanRate:      constant(2048.0),
+                       growthRate:    constant(2.0).sum(ramp(-1.0, 8)),
+                       scannableFrac: constant(0.01),
+                       stackBytes:    constant(8192),
+                       length:        50,
+                       checker: func(t *testing.T, c []gcCycleResult) {
+                               n := len(c)
+                               if n > 9 {
+                                       // After the 4th GC, the heap will stop growing.
+                                       // First, let's make sure we're finishing near the goal, with some extra
+                                       // room because we're probably going to be triggering early.
+                                       assertInRange(t, "goal ratio", c[n-1].goalRatio(), 0.925, 1.025)
+                                       // Next, let's make sure there's some minimum distance between the goal
+                                       // and the trigger. It should be proportional to the runway (hence the
+                                       // trigger ratio check, instead of a check against the runway).
+                                       assertInRange(t, "trigger ratio", c[n-1].triggerRatio(), 0.925, 0.975)
+                               }
+                               if n > 25 {
+                                       // Double-check that GC utilization looks OK.
+
+                                       // At this alloc/scan rate, the pacer should be extremely close to the goal utilization.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, GCGoalUtilization, 0.005)
+                                       // Make sure GC utilization has mostly levelled off.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[n-2].gcUtilization, 0.05)
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[11].gcUtilization, 0.05)
+                               }
+                       },
+               },
+               {
+                       // This test sets a slow allocation rate and a large heap to try to minimize the
+                       // difference between the trigger and the goal.
+                       name:          "LargeHeapSlowAlloc",
+                       gcPercent:     100,
+                       globalsBytes:  32 << 10,
+                       nCores:        8,
+                       allocRate:     constant(1.0),
+                       scanRate:      constant(2048.0),
+                       growthRate:    constant(4.0).sum(ramp(-3.0, 12)),
+                       scannableFrac: constant(0.01),
+                       stackBytes:    constant(8192),
+                       length:        50,
+                       checker: func(t *testing.T, c []gcCycleResult) {
+                               n := len(c)
+                               if n > 13 {
+                                       // After the 4th GC, the heap will stop growing.
+                                       // First, let's make sure we're finishing near the goal.
+                                       assertInRange(t, "goal ratio", c[n-1].goalRatio(), 0.95, 1.05)
+                                       // Next, let's make sure there's some minimum distance between the goal
+                                       // and the trigger. It should be around the default minimum heap size.
+                                       assertInRange(t, "runway", c[n-1].runway(), DefaultHeapMinimum-64<<10, DefaultHeapMinimum+64<<10)
+                               }
+                               if n > 25 {
+                                       // Double-check that GC utilization looks OK.
+
+                                       // At this alloc/scan rate, the pacer should be extremely close to the goal utilization.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, GCGoalUtilization, 0.005)
+                                       // Make sure GC utilization has mostly levelled off.
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[n-2].gcUtilization, 0.05)
+                                       assertInEpsilon(t, "GC utilization", c[n-1].gcUtilization, c[11].gcUtilization, 0.05)
+                               }
+                       },
+               },
                // TODO(mknyszek): Write a test that exercises the pacer's hard goal.
                // This is difficult in the idealized model this testing framework places
                // the pacer in, because the calculated overshoot is directly proportional
@@ -532,6 +639,14 @@ func (r *gcCycleResult) goalRatio() float64 {
        return float64(r.heapPeak) / float64(r.heapGoal)
 }
 
+func (r *gcCycleResult) runway() float64 {
+       return float64(r.heapGoal - r.heapTrigger)
+}
+
+func (r *gcCycleResult) triggerRatio() float64 {
+       return float64(r.heapTrigger-r.heapLive) / float64(r.heapGoal-r.heapLive)
+}
+
 func (r *gcCycleResult) String() string {
        return fmt.Sprintf("%d %2.1f%% %d->%d->%d (goal: %d)", r.cycle, r.gcUtilization*100, r.heapLive, r.heapTrigger, r.heapPeak, r.heapGoal)
 }