Skip to content

runtime: remove idle GC workers for the 2-minute GC cycle #44163

Closed

Description

@mknyszek

Background

Today the Go GC will spin up dedicated GC workers which use 25% of GOMAXPROCS resources. But it will also soak up any additional GOMAXPROCS that are idle and run "idle GC workers" to do so.

The idea here is that those cores are idle anyway, so why not use them to speed up the GC cycle? There are negative performance implications to having the GC on longer than necessary (floating garbage accumulation for memory, and write barrier enabled for CPU time).

EDIT: To be clear, I'm not proposing removing dedicated GC workers, which use 25% of CPU resources during a GC cycle. Those are very, very important. Just the idle workers that can increase that number to 100% if the application is idle.

Why?

When a Go process is mostly idle, idle enough that the "every-2-minutes" GC kicks in, it's going to go full blast on all CPU cores, resulting in a CPU utilization spike. While this on it's own isn't a big deal, it makes a difference in the face of monitoring systems and auto-scaling systems. The former is surprising; one might see these spikes in their monitoring and spend time investigating them, only to find there's nothing to do about that (today). The latter is dangerous: if an auto-scaling system decides to give an application more resources because it notices this spike, then the Go GC is now going to soak up those resources, resulting in a significant (if temporary, and generally bounded) increase in resources.

From the earlier proposal to remove idle GC workers (no longer relevant):

I believe today these idle GC workers are doing more harm than good.

Firstly, when a Go process is mostly idle, idle enough that the "every-2-minutes" GC kicks in, it's going to go full blast on all CPU cores, resulting in a CPU utilization spike. While this on it's own isn't a big deal, it makes a difference in the face of monitoring systems and auto-scaling systems. The former is surprising; one might see these spikes in their monitoring and spend time investigating them, only to find there's nothing to do about that (today). The latter is dangerous: if an auto-scaling system decides to give an application more resources because it notices this spike, then the Go GC is now going to soak up those resources, resulting in a significant (if temporary, and generally bounded) increase in resources.

Secondly, the Go pacer fails to account for this extra CPU utilization. This significantly skews the pacer's function and makes it consistently undershoot its target in the steady-state. The goal of the pacer is to reach some GC CPU utilization target, but that target is somewhat meaningless if the idle GC workers can kick in and blow that away in applications that have any idle time. The result is that if idle GC workers are running, the GC will consistently undershoot its goal.

Worse still, this extra CPU utilization isn't easy to account for in the pacer. If we pace assuming the idle GC workers are running, it's possible for the GC to really panic and spike utilization just when you really need it. For instance, take an application with many goroutines blocked on syscalls. Let's say most of the time they're not doing a lot, but suddenly (by chance) many wake up at once during a GC cycle. Suddenly the GC CPU utilization will drop (no space for idle workers) and the GC will think it's extremely behind, so at that moment it'll start forcing all these goroutines to assist, basically up to the utilization that the idle workers were doing. This makes our notion of a steady-state fragile, since it now depends on the CPU resources available. (I have a new pacer design in the works and while it can deal with this, it ends up with consistent undershoot when the idle workers are running; I don't think you can win here.)

Thirdly, there's a latency cost. Consider again the case of goroutines waiting on, say, network requests. The Go scheduler is going to schedule idle GC workers, and while they yield readily (they're "low priority" in a sense), their function is primarily non-preemptible and likely will be for the forseeable future. The cost of scheduling them away is small, but not free.

Why not?

The application could suddenly become not idle during the GC cycle, and there would be a latency cost in the form of assists (vs. idle GC workers, which tend to introduce very little latency cost). Given that the 2-minute GC kicks in, however, I really doubt that this will every be an issue. The application's steady-state would have to change mid-GC, and on the time scale of 2 minutes, that's already extremely unlikely.

From the earlier proposal to remove idle GC workers (no longer relevant):

I'm honestly not sure. The only scenario I could construct where the idle GC workers come in handy is a bursty application, since you increase your chance that the GC fires outside of a burst, and you get a nice boost in throughput and latency since the GC conveniently tries to finish as fast as possible outside the burst.

But this argument is still somewhat dubious because there's no guarantee the GC won't just fire during the burst, so I'm not sure how much this actually helps. However, one reason I'm creating this issue is that maybe someone out there can think of another scenario, or has another use-case.

Why not now?

I don't think there's any reason not to do this very targeted fix. It will require some additional testing to make sure everything still works correctly, because the 2-minute GC is a fairly rare case, so bugs here will manifest as surprising and very rare failures.

From the earlier proposal to remove idle GC workers (no longer relevant):

Removing the idle GC workers is unfortunately going to be risky from an implementation standpoint. @prattmic tried this out and unfortunately it uncovered at least one bug. It's possible that over time the idle GC workers have become critical to GC progress in some scenarios. Oops.

@prattmic concluded from his investigation into this that it's going to uncover a non-trivial amount of latent, subtle bugs (and I say bugs since theoretically the idle GC workers should have no effect on the function of the GC otherwise) and it's going to take a while to squash all of them.

As a result, it's going to be critical to land this early in the development cycle and give it plenty of time to soak, with an easy roll-back option (perhaps a runtime-internal constant).

However, despite all this, I think this is worth doing sooner rather than later. The longer we wait, the greater chance we have that seemingly orthogonal changes to the GC tangle this more.

CC @aclements @prattmic

Activity

  1. added
    NeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.
    on Feb 8, 2021
  2. added this to the Go1.17 milestone on Feb 8, 2021
  3. randall77 commented on Feb 8, 2021

    @randall77
    Contributor

    So who's doing the GC work then? Is it all handled during assists?

    If so, then I worry about an application that stops allocating mid-cycle while the write barrier is active. Then is the write barrier active forever? Is all the floating garbage never collected (and resulting free pages returned to the OS)?

    Or is some of the GC work done during write barriers also? That would eliminate the write-barrier-on-forever problem, and lessen, but maybe not eliminate, the floating garbage one (to still have a problem you'd need an application that runs but never writes a pointer).

  4. mknyszek commented on Feb 8, 2021

    @mknyszek
    ContributorAuthor

    @randall77 I only mean idle GC workers (which are dedicated low-priority workers that soak up additional idle Ps in addition to the 25% dedicated), not the 25% dedicated workers. I think it's very important to keep dedicated workers, for the reasons you mention (and others).

  5. mknyszek commented on Apr 15, 2021

    @mknyszek
    ContributorAuthor

    Definitely not in this release, but I've found some other issues with dedicated worker scheduling in working on #44167. I would like to address all these issues in the final implementation for #44167 which is currently in progress.

  6. modified the milestones: Go1.17, Go1.18 on Apr 15, 2021
  7. self-assigned this
    on Apr 19, 2021
  8. added
    early-in-cycleA change that should be done early in the 3 month dev cycle.
    on Apr 23, 2021
  9. cagedmantis commented on Aug 12, 2021

    @cagedmantis
    Contributor

    This issue is currently labeled as early-in-cycle for Go 1.18.
    That time is now, so this is a friendly ping so the issue is looked at again.

  10. mknyszek commented on Aug 12, 2021

    @mknyszek
    ContributorAuthor

    thanks @cagedmantis, I'm preparing a change for this, it's on my radar.

  11. mknyszek commented on Aug 12, 2021

    @mknyszek
    ContributorAuthor

    ah, though I will note I probably won't land it before the tree opens. I don't see a need.

  12. gopherbot commented on Sep 20, 2021

    @gopherbot
    Contributor

    Change https://golang.org/cl/351131 mentions this issue: runtime: add an experiment to disable idle GC mark workers

  13. aclements commented on Oct 5, 2021

    @aclements
    Member

    Bumping to 1.19 early-in-cycle.

  14. 12 remaining items

  15. mknyszek commented on Mar 21, 2022

    @mknyszek
    ContributorAuthor

    I think timers could work. Two potential problems, though, from experience using timers for the scavenger:

    1. It'll delay checkdead unless we're careful about how the goroutine is parked and unparked. This is mainly a test issue.
    2. The granularity of our timers is pretty low. The scavenger (also paced by CPU utilization) needs to run for about 1 ms before producing a sleep duration large enough to be consistently correlated with the input sleep duration. I fear chaotic changes in CPU usage for GOMAXPROCS<6 if we try to schedule at a higher granularity.

    (1) is probably a non-issue, since a GC cycle has a pretty short duration compared to completing all the scavenge work, and checkdead typically isn't all that useful once the application gets big enough to have a long GC cycle. We can also work around this, it's just complicated.

    I have honestly no idea whether (2) will be a problem in practice. It depends a lot on the length of the GC cycle. The shortest GC cycles can definitely be < 1 ms in length.

    As a result, I don't think this is just a striaghtforward "let's do it this way," though I can be convinced. Currently, I think it needs a bit of experimentation and tuning, which I'll fold into my "fundamentally reworking how fractional workers operate" comment above. :)

  16. prattmic commented on Mar 21, 2022

    @prattmic
    Member

    What if the problem isn't that we need idle workers to ensure progress, but just fractional workers need to be less special? If the fractional worker used a timer for when it needed to run again rather than relying on the scheduler to poll for if it needs to run, then it would be able to wake a stopped M.

    I agree this would be nice. In fact, it would be nice if dedicated and fractional workers were closer to "normal" goroutines that could simply move to _Grunnable when they need to run (with the scheduler boosting their priority I guess? Haven't thought about this too much).

    Working with timers will be a bit awkward since there isn't an obvious time we need to run again, we first need to pick a period to use. To Michael's point about timer granularity, perhaps we could have a pretty long period. e.g., fractional workers will run for at least 10ms before sleeping. On the other hand, this means a 25% fractional worker will sleep for 30ms, which while not technically deadlocked may be a long time to wait if all other goroutines are blocked waiting for GC to make progress.

    The scavenger (also paced by CPU utilization) needs to run for about 1 ms before producing a sleep duration large enough to be consistently correlated with the input sleep duration

    I'm not sure precisely what you mean, but this just sounds like a description of #44343.

  17. mknyszek commented on Mar 21, 2022

    @mknyszek
    ContributorAuthor

    On the other hand, this means a 25% fractional worker will sleep for 30ms, which while not technically deadlocked may be a long time to wait if all other goroutines are blocked waiting for GC to make progress.

    That's a good point. In theory if the application is active, this is fine; assists will finish off the GC. Another problem with a long period is something like GOMAXPROCS=1: if the GC is short enough, then 10 ms could easily just eat up the entire GC cycle each time, effectively turning into a STW GC (nothing makes any progress except the GC). I realize 10 ms was just your strawman number, but even 1 ms could have the same effect. We can deal with this a bit by adding a bit of random delay before the fractional worker kicks in, but it's just more complexity.

    At first glance this might not seem any worse than idle mark workers eating up an entire scheduling quantum, but idle mark workers only pop up when there's actually nothing to do.

    I'm not sure precisely what you mean, but this just sounds like a description of #44343.

    That's correct.

    (And more precisely, I was referring to the scavenger's controller, which assumes that sleeping longer has some kind of proportional effect on CPU utilization. At smaller sleep increments than 1 ms, the amount actually slept varies wildly enough that error in the controller accumulates in an unbounded manner.)

    I think your comments serve my broader point that to "fix" the fractional worker would require (and certainly benefit from) a rethink of how GC scheduling works today. While I think that's worthwhile, I'd also like to offer my preference to not block this issue on that.

    That being said, if y'all don't like this interim solution of just limiting idle mark workers (turning them down to zero if we have at least one dedicated mark worker), I'm willing to put this aside and find time later to look into making this work better.

  18. prattmic commented on Mar 21, 2022

    @prattmic
    Member

    That being said, if y'all don't like this interim solution of just limiting idle mark workers (turning them down to zero if we have at least one dedicated mark worker), I'm willing to put this aside and find time later to look into making this work better.

    I'm OK with it. IMO, the whole discussion above is an argument in favor of having some kind of idle mark workers. At least in the (admittedly rare) case of a program blocked on GC completion it is difficult to argue that you wouldn't want at least one worker running as soon as the entire program is idle.

  19. aclements commented on Mar 21, 2022

    @aclements
    Member

    I'm fine with the solution of limiting the idle workers, too. I just wanted to make sure we'd thought through the problem.

  20. gopherbot commented on Mar 24, 2022

    @gopherbot
    Contributor

    Change https://go.dev/cl/395634 mentions this issue: runtime: disable idle mark workers with at least one dedicated worker

  21. moved this from Todo to In Progress in Go Compiler / Runtime / Commandon Mar 28, 2022
  22. Repository owner moved this from In Progress to Done in Go Compiler / Runtime / Commandon Apr 26, 2022
  23. locked and limited conversation to collaborators on Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeGarbageCollectorNeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.early-in-cycleA change that should be done early in the 3 month dev cycle.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions