runtime: long latency of sweep assists [1.20 backport] #58536
Labels
CherryPickApproved
Used during the release process for point releases
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
Milestone
Comments
gopherbot
added
the
CherryPickCandidate
gopherbot
added
the
compiler/runtime
|
Change https://go.dev/cl/468375 mentions this issue: |
|
I want to amend the backport reasoning slightly to point out that it's specifically a combination of |
prattmic
added
the
CherryPickApproved
gopherbot
removed
the
CherryPickCandidate
gopherbot
pushed a commit
that referenced
this issue
Feb 15, 2023
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>
|
Closed by merging 7b398b1 to release-branch.go1.20. |
romaindoumenc
pushed a commit
to TroutSoftware/go
that referenced
this issue
Mar 3, 2023
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 golang#57523. Fixes golang#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>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
@mknyszek requested issue #57523 to be considered for backport to the next 1.20 minor release.
The text was updated successfully, but these errors were encountered: