diff options
| author | Michael Anthony Knyszek <mknyszek@google.com> | 2021-11-04 21:09:34 +0000 |
|---|---|---|
| committer | Michael Knyszek <mknyszek@google.com> | 2021-11-04 21:41:49 +0000 |
| commit | 156abe51221c5723c3ff524ea0fcbe65d8272bfa (patch) | |
| tree | 418f208d07ef8d7bce29a523cdc3a212454b3921 | |
| parent | 2c32f29f2f88e56f329547467090e7315cd3c1e8 (diff) | |
| download | go-156abe51221c5723c3ff524ea0fcbe65d8272bfa.tar.gz go-156abe51221c5723c3ff524ea0fcbe65d8272bfa.zip | |
runtime: fix hard goal calculation
The new GC pacer has a bug where the hard goal isn't set in relation to
the original heap goal, but rather to the one already extrapolated for
overshoot.
In practice, I have never once seen this case arise because the
extrapolated goal used for overshoot is conservative. No test because
writing a test for this case is impossible in the idealized model the
pacer tests create. It is possible to simulate but will take more work.
For now, just leave a TODO.
Change-Id: I24ff710016cd8100fad54f71b2c8cdea0f7dfa79
Reviewed-on: https://go-review.googlesource.com/c/go/+/361435
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
| -rw-r--r-- | src/runtime/mgcpacer.go | 7 | ||||
| -rw-r--r-- | src/runtime/mgcpacer_test.go | 6 |
2 files changed, 10 insertions, 3 deletions
diff --git a/src/runtime/mgcpacer.go b/src/runtime/mgcpacer.go index f886a07da1..230e78b000 100644 --- a/src/runtime/mgcpacer.go +++ b/src/runtime/mgcpacer.go @@ -517,7 +517,7 @@ func (c *gcControllerState) revise() { // growths. It's OK to use more memory this cycle to scan all the live heap, // because the next GC cycle is inevitably going to use *at least* that much // memory anyway. - heapGoal = int64(float64(heapGoal-int64(c.trigger))/float64(scanWorkExpected)*float64(maxScanWork)) + int64(c.trigger) + extHeapGoal := int64(float64(heapGoal-int64(c.trigger))/float64(scanWorkExpected)*float64(maxScanWork)) + int64(c.trigger) scanWorkExpected = maxScanWork // hardGoal is a hard limit on the amount that we're willing to push back the @@ -528,9 +528,10 @@ func (c *gcControllerState) revise() { // This maintains the invariant that we use no more memory than the next GC cycle // will anyway. hardGoal := int64((1.0 + float64(gcPercent)/100.0) * float64(heapGoal)) - if heapGoal > hardGoal { - heapGoal = hardGoal + if extHeapGoal > hardGoal { + extHeapGoal = hardGoal } + heapGoal = extHeapGoal } if int64(live) > heapGoal { // We're already past our heap goal, even the extrapolated one. diff --git a/src/runtime/mgcpacer_test.go b/src/runtime/mgcpacer_test.go index d2707ca5a1..9ec0e5172b 100644 --- a/src/runtime/mgcpacer_test.go +++ b/src/runtime/mgcpacer_test.go @@ -302,6 +302,12 @@ func TestGcPacer(t *testing.T) { } }, }, + // 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 + // to the runway for the case of the expected work. + // However, it is still possible to trigger this case if something exceptional + // happens between calls to revise; the framework just doesn't support this yet. } { e := e t.Run(e.name, func(t *testing.T) { |