| Age | Commit message (Collapse) | Author |
|---|
|
This change reduces the maximum number of idle mark workers during
periodic (currently every 2 minutes) GC cycles to 1.
Idle mark workers soak up all available and unused Ps, up to GOMAXPROCS.
While this provides some throughput and latency benefit in general, it
can cause what appear to be massive CPU utilization spikes in otherwise
idle applications. This is mostly an issue for *very* idle applications,
ones idle enough to trigger periodic GC cycles. This spike also tends to
interact poorly with auto-scaling systems, as the system might assume
the load average is very low and suddenly see a massive burst in
activity.
The result of this change is not to bring down this 100% (of GOMAXPROCS)
CPU utilization spike to 0%, but rather
min(25% + 1/GOMAXPROCS*100%, 100%)
Idle mark workers also do incur a small latency penalty as they must be
descheduled for other work that might pop up. Luckily the runtime is
pretty good about getting idle mark workers off of Ps, so in general
the latency benefit from shorter GC cycles outweighs this cost. But, the
cost is still non-zero and may be more significant in idle applications
that aren't invoking assists and write barriers quite as often.
We can't completely eliminate idle mark workers because they're
currently necessary for GC progress in some circumstances. Namely,
they're critical for progress when all we have is fractional workers. If
a fractional worker meets its quota, and all user goroutines are blocked
directly or indirectly on a GC cycle (via runtime.GOMAXPROCS, or
runtime.GC), the program may deadlock without GC workers, since the
fractional worker will go to sleep with nothing to wake it.
Fixes #37116.
For #44163.
Change-Id: Ib74793bb6b88d1765c52d445831310b0d11ef423
Reviewed-on: https://go-review.googlesource.com/c/go/+/393394
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Now that Go 1.18 has been released, remove the old pacer.
Change-Id: Ie7a7596d67f3fc25d3f375a08fc75eafac2eb834
Reviewed-on: https://go-review.googlesource.com/c/go/+/393396
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
If something goes horribly wrong with the assumptions surrounding a
piController, its internal error state might accumulate in an unbounded
manner. In practice this means unexpected Inf and NaN values.
Avoid this by identifying cases where the error overflows and resetting
controller state.
In the scavenger, this case is much more likely. All that has to happen
is the proportional relationship between sleep time and estimated CPU
usage has to break down. Unfortunately because we're just measuring
monotonic time for all this, there are lots of ways it could happen,
especially in an oversubscribed system. In these cases, just fall back
on a conservative pace for scavenging and try to wait out the issue.
In the pacer I'm pretty sure this is impossible. Because we wire the
output of the controller to the input, the response is very directly
correlated, so it's impossible for the controller's core assumption to
break down.
While we're in the pacer, add more detail about why that controller is
even there, as well as its purpose.
Finally, let's be proactive about other sources of overflow, namely
overflow from a very large input value. This change adds a check after
the first few operations to detect overflow issues from the input,
specifically the multiplication.
No tests for the pacer because I was unable to actually break the
pacer's controller under a fuzzer, and no tests for the scavenger because
it is not really in a testable state.
However:
* This change includes a fuzz test for the piController.
* I broke out the scavenger code locally and fuzz tested it, confirming
that the patch eliminates the original failure mode.
* I tested that on a local heap-spike test, the scavenger continues
operating as expected under normal conditions.
Fixes #51061.
Change-Id: I02a01d2dbf0eb9d2a8a8e7274d4165c2b6a3415a
Reviewed-on: https://go-review.googlesource.com/c/go/+/383954
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
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>
|
|
This change implements the GC pacer redesign outlined in #44167 and the
accompanying design document, behind a GOEXPERIMENT flag that is on by
default.
In addition to adding the new pacer, this CL also includes code to track
and account for stack and globals scan work in the pacer and in the
assist credit system.
The new pacer also deviates slightly from the document in that it
increases the bound on the minimum trigger ratio from 0.6 (scaled by
GOGC) to 0.7. The logic behind this change is that the new pacer much
more consistently hits the goal (good!) leading to slightly less
frequent GC cycles, but _longer_ ones (in this case, bad!). It turns out
that the cost of having the GC on hurts throughput significantly (per
byte of memory used), though tail latencies can improve by up to 10%! To
be conservative, this change moves the value to 0.7 where there is a
small improvement to both throughput and latency, given the memory use.
Because the new pacer accounts for the two most significant sources of
scan work after heap objects, it is now also safer to reduce the minimum
heap size without leading to very poor amortization. This change thus
decreases the minimum heap size to 512 KiB, which corresponds to the
fact that the runtime has around 200 KiB of scannable globals always
there, up-front, providing a baseline.
Benchmark results: https://perf.golang.org/search?q=upload:20211001.6
tile38's KNearest benchmark shows a memory increase, but throughput (and
latency) per byte of memory used is better.
gopher-lua showed an increase in both CPU time and memory usage, but
subsequent attempts to reproduce this behavior are inconsistent.
Sometimes the overall performance is better, sometimes it's worse. This
suggests that the benchmark is fairly noisy in a way not captured by the
benchmarking framework itself.
biogo-igor is the only benchmark to show a significant performance loss.
This benchmark exhibits a very high GC rate, with relatively little work
to do in each cycle. The idle mark workers are quite active. In the new
pacer, mark phases are longer, mark assists are fewer, and some of that
time in mark assists has shifted to idle workers. Linux perf indicates
that the difference in CPU time can be mostly attributed to write-barrier
slow path related calls, which in turn indicates that the write barrier
being on for longer is the primary culprit. This also explains the memory
increase, as a longer mark phase leads to more memory allocated black,
surviving an extra cycle and contributing to the heap goal.
For #44167.
Change-Id: I8ac7cfef7d593e4a642c9b2be43fb3591a8ec9c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/309869
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
This change creates a formal exported interface for the GC pacer and
creates tests for it that simulate some series of GC cycles. The tests
are completely driven by the real pacer implementation, except for
assists, which are idealized (though revise is called repeatedly).
For #44167.
Change-Id: I0112242b07e7702595ca71001d781ad6c1fddd2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/353354
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
