qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 01:54:45 -07:00
Code Releases Activity

runtime: check for overflow in sweep assist

Browse Source 
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.

Fixes #57523.

Change-Id: Ib18f71f53468e913548aac6e5358830c72ef0215
Reviewed-on: https://go-review.googlesource.com/c/go/+/460376
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
Michael Anthony Knyszek 2023-01-04 05:20:58 +00:00 committed by Gopher Robot
parent e2b620ce73
commit a7b75972f2
1 changed files with 22 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/runtime/mgcsweep.go
View File

@ -873,11 +873,30 @@ func deductSweepCredit(spanBytes uintptr, callerSweepPages uintptr) {
traceGCSweepStart()
}
// Fix debt if necessary.
retry:
sweptBasis := mheap_.pagesSweptBasis.Load()
// Fix debt if necessary.
newHeapLive := uintptr(gcController.heapLive.Load()-mheap_.sweepHeapLiveBasis) + spanBytes
live := gcController.heapLive.Load()
liveBasis := mheap_.sweepHeapLiveBasis
newHeapLive := spanBytes
if liveBasis < live {
// Only do this subtraction when we don't overflow. Otherwise, pagesTarget
// might be computed as something really huge, causing us to get stuck
// sweeping here until the next mark phase.
//
// Overflow can happen here if gcPaceSweeper is called concurrently with
// sweeping (i.e. not during a STW, like it usually is) because this code
// is intentionally racy. A concurrent call to gcPaceSweeper can happen
// if a GC tuning parameter is modified and we read an older value of
// heapLive than what was used to set the basis.
//
// This state should be transient, so it's fine to just let newHeapLive
// be a relatively small number. We'll probably just skip this attempt to
// sweep.
//
// See issue #57523.
newHeapLive += uintptr(live - liveBasis)
}
pagesTarget := int64(mheap_.sweepPagesPerByte*float64(newHeapLive)) - int64(callerSweepPages)
for pagesTarget > int64(mheap_.pagesSwept.Load()-sweptBasis) {
if sweepone() == ^uintptr(0) {