languages/go
languages/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-10-02 22:25:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

runtime: revert to using the precomputed trigger for pacer calculations

Browse source 
Issue #53738 describes in detail how switching to using the actual
trigger point over the precomputed trigger causes a memory regression,
that arises from the fact that the PI controller in front of the
cons/mark ratio has a long time constant (for overdamping), so it
retains a long history of inputs.

This change, for the Go 1.19 cycle, just reverts to using the
precomputed trigger because it's safer, but in the future we should
consider moving away from such a history-sensitive smoothing function.

See the big comment in the diff and #53738 for more details.

Performance difference vs. 1.18 after this change:
https://perf.golang.org/search?q=upload:20220714.15

Fixes #53738.

Change-Id: I636993a730a3eaed25da2a2719860431b296c6f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/417557
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
Michael Anthony Knyszek 2022-07-14 21:19:37 +00:00 committed by Michael Knyszek
parent ae7340ab68
commit 85a482fc24
1 changed files with 20 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
src/runtime/mgcpacer.go
View file

@ -439,7 +439,26 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
c.fractionalMarkTime = 0
c.idleMarkTime = 0
c.markStartTime = markStartTime
c.triggered = c.heapLive
// TODO(mknyszek): This is supposed to be the actual trigger point for the heap, but
// causes regressions in memory use. The cause is that the PI controller used to smooth
// the cons/mark ratio measurements tends to flail when using the less accurate precomputed
// trigger for the cons/mark calculation, and this results in the controller being more
// conservative about steady-states it tries to find in the future.
//
// This conservatism is transient, but these transient states tend to matter for short-lived
// programs, especially because the PI controller is overdamped, partially because it is
// configured with a relatively large time constant.
//
// Ultimately, I think this is just two mistakes piled on one another: the choice of a swingy
// smoothing function that recalls a fairly long history (due to its overdamped time constant)
// coupled with an inaccurate cons/mark calculation. It just so happens this works better
// today, and it makes it harder to change things in the future.
//
// This is described in #53738. Fix this for #53892 by changing back to the actual trigger
// point and simplifying the smoothing function.
heapTrigger, heapGoal := c.trigger()
c.triggered = heapTrigger
// Compute the background mark utilization goal. In general,
// this may not come out exactly. We round the number of
@ -501,7 +520,6 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
c.revise()
if debug.gcpacertrace > 0 {
heapGoal := c.heapGoal()
assistRatio := c.assistWorkPerByte.Load()
print("pacer: assist ratio=", assistRatio,
" (scan ", gcController.heapScan>>20, " MB in ",