Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: runtime/debug: user-configurable memory target #44309

Closed
mknyszek opened this issue Feb 16, 2021 · 4 comments
Closed

proposal: runtime/debug: user-configurable memory target #44309

mknyszek opened this issue Feb 16, 2021 · 4 comments

Comments

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Feb 16, 2021

User-configurable memory target

Author: Michael Knyszek

Abstract

Issue #23044 proposed the addition of some kind of API to provide a "minimum heap" size; that is, the minimum heap goal that the GC would ever set. The purpose of a minimum heap size, as explored in that proposal, is as a performance optimization: by preventing the heap from shrinking, each GC cycle will get longer as the live heap shrinks further beyond the minimum. Today, this performance optimization is achieved using a heap ballast.

This proposal builds on #44167 as it resolves key issues with the pacer that blocked consideration of #23044. The core proposal in #23044 is now straightforward to implement. The only thing we need is evidence that it actually will work, a justification for the new "knob," and a concrete API.

Evidence.
Using the same simulation framework to validate #44167, I am able to show that this design behaves well in a number of scenarios.

Justification.
Users are already using heap ballasts, so we're indirectly already supporting this optimization. Unfortunately heap ballasts are also platform-dependent, because they rely on OS-level memory accounting being lazy, which isn't true on a number of platforms such as Windows. By formalizing this behavior as an API, we give users the same amount of control of performance but in a safer, backwards compatible, and cross-platform way.

The biggest risk of this proposal is the introduction of a new "GC knob," as that may lead to an explosion in the number of possible GC configurations. Testing and maintaining all of them is very difficult, and other GCs just give up and only support a small subset of configurations. I believe that my API formulation bounds the number of possible GC configurations, and my implementation plans for #44167 (that is, making the pacer in the runtime testable) limit this risks of having many more potential GC configurations enough to be worth it. That isn't to say we shouldn't be judicious of adding new GC knobs going forward, but this is one that has shown to be a clear user need over the years.

API.
I propose a runtime API similar to runtime/debug.SetGCPercent called runtime/debug.SetMemoryTarget. It provides the runtime a hint that it is free to ignore, but in practice will use to execute fewer GC cycles in exchange for using up to the target amount of memory.

Full design

Found here.

Credit goes to @cespare for proposing this in the first place.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 16, 2021

Change https://golang.org/cl/292789 mentions this issue: design: add user-configurable memory target

Loading

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Feb 17, 2021
@matttproud
Copy link
Contributor

@matttproud matttproud commented Feb 17, 2021

I have reviewed the design proposal. From an overall reasoning, ecosystem and production fitness perspective, and most specifically a process execution plan standpoint, I support it and think it is sound. Thank you for all of the hard work you've put into it.

Can't wait to see what you folks come up with on the other efforts this dovetails!

Loading

gopherbot pushed a commit to golang/proposal that referenced this issue Feb 17, 2021
For golang/go#44309.

Change-Id: Ibd2f9bed3a1a1da40b5a3d216ccb1f48c9b64c04
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/292789
Reviewed-by: Michael Pratt <mpratt@google.com>
@mknyszek
Copy link
Contributor Author

@mknyszek mknyszek commented Apr 15, 2021

Given that I pushed #44167 back, I definitely need to push this back.

Loading

@mknyszek mknyszek removed this from the Go1.17 milestone Apr 15, 2021
@mknyszek mknyszek added this to the Go1.18 milestone Apr 15, 2021
@dmitshur dmitshur removed this from the Go1.18 milestone Nov 10, 2021
@dmitshur dmitshur added this to the Proposal milestone Nov 10, 2021
@mknyszek
Copy link
Contributor Author

@mknyszek mknyszek commented Nov 10, 2021

Given that #48409 has been accepted, and I haven't received any feedback (as far as I can tell) that this would better suit some use-case, I'm going to withdraw this proposal.

Loading

@mknyszek mknyszek closed this Nov 10, 2021
@ianlancetaylor ianlancetaylor removed this from Incoming in Proposals Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants