]> Cypherpunks repositories - gostls13.git/commit
[release-branch.go1.20] runtime: check for overflow in sweep assist
authorMichael Anthony Knyszek <mknyszek@google.com>
Wed, 4 Jan 2023 05:20:58 +0000 (05:20 +0000)
committerMichael Pratt <mpratt@google.com>
Wed, 15 Feb 2023 21:52:31 +0000 (21:52 +0000)
commit7b398b1ff7d751032ea1e4ffed6f02675fc51c3b
treef0cecffc07e02d1b72e1bbf8f49efb4b8a287c47
parent2d01f3695becd725959a723b512cf46e88ae60fe
[release-branch.go1.20] runtime: check for overflow in sweep assist

The sweep assist computation is intentionally racy for performance,
since the specifics of sweep assist aren't super sensitive to error.
However, if overflow occurs when computing the live heap delta, we can
end up with a massive sweep target that causes the sweep assist to sweep
until sweep termination, causing severe latency issues. In fact, because
heapLive doesn't always increase monotonically then anything that
flushes mcaches will cause _all_ allocating goroutines to inevitably get
stuck in sweeping.

Consider the following scenario:
1. SetGCPercent is called, updating sweepHeapLiveBasis to heapLive.
2. Very shortly after, ReadMemStats is called, flushing mcaches and
   decreasing heapLive below the value sweepHeapLiveBasis was set to.
3. Every allocating goroutine goes to refill its mcache, calls into
   deductSweepCredit for sweep assist, and gets stuck sweeping until
   the sweep phase ends.

Fix this by just checking for overflow in the delta live heap calculation
and if it would overflow, pick a small delta live heap. This probably
means that no sweeping will happen at all, but that's OK. This is a
transient state and the runtime will recover as soon as heapLive
increases again.

Note that deductSweepCredit doesn't check overflow on other operations
but that's OK: those operations are signed and extremely unlikely to
overflow. The subtraction targeted by this CL is only a problem because
it's unsigned. An alternative fix would be to make the operation signed,
but being explicit about the overflow situation seems worthwhile.

For #57523.
Fixes #58536.

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