-
Notifications
You must be signed in to change notification settings - Fork 21.2k
internal/debug: add debug_setMemoryLimit #31441
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
internal/debug: add debug_setMemoryLimit #31441
Conversation
Some updates: GOGC=500: https://gist.github.com/georgehao/ce60f675699bbed77b988446e296c197 The average gc interval:
This means using large GOGC will enlarge the gc cycle and let the CPU not cost too much on Golang GC. Actually, it doesn't mean the bigger GOGC is better, we should support a way to optimize. |
From my understanding, your main requirement is a way to adjust the GOGC value,
This allows you to bypass the startup limitations. For several reasons, we prefer not to support a high GOGC value by default. |
Regarding the SetMemoryLimit API, we prefer not to use it. This API imposes a hard cap |
I don't quite agree with you.
|
@rjl493456442 hope you can review it again, thx |
I also agree don't use a large GOGC, this should to decide by the realistic env. the default 100 is the recommendation. but current geth's implementation: with large machine memory, the gc will become very small. the min value is 20. I think this is too small, it will trigger gc very frequently and lock the application (you will find the gcMaker will take 1/3 of the pprof this PR's change:
These two changes make it possible to optimize for their own environment based on their env (If the user has the ability to optimize). And this also will don't affect the current logic. |
@rjl493456442, what's your opinion? |
@rjl493456442 fixed |
// SetMemoryLimit sets the GOMEMLIMIT for the process. It returns the previous limit. | ||
// Note: | ||
// - Geth also allocates memory off-heap, particularly for fastCache and Pebble, which can be non-trivial (a few gigabytes by default). | ||
// - Setting the limit too low will cause Geth to become unresponsive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you at least add a comment stating that the input limit is specified in bytes? I’m not sure whether we should sanitize excessively low values, as it’s unclear where the threshold should be.
But I can foresee that people might pass the values as the megabytes or so, resulting in an unexpected behavior.
I would also prefer to print out some logs so that users know what's the threshold being set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply. I will submit as soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure whether we should sanitize excessively low values, as it’s unclear where the threshold should be.
The minimum value is not easy to decide, I added a comment if the setting is too small
@rjl493456442 is there anything need to update? |
when send too many transaction to txpool during my benchmark, the golang gc will take 1/3 CPU profile.
I want to use the
ballast
mechanism to reduce the gc times to reduce golang GC CPU cost. I try GOGC = 500 and GOMEMLIMIT=10G, but it doesn't work. mainly because this section of code limits the GOGC to never be over 100.go-ethereum/cmd/utils/flags.go
Lines 1583 to 1597 in 03cc294
Assuming the memory is 30G, GOGC value is 20 after the calculation. It also means the larger the memory, the smaller the gogc. This will make the Golang GC very frequent, wasting the CPU resources, so suggest removing this part.
If we wonder about the large number of txs that trigger the OOM, the suggestion is to set GOMEMLIMIT depending on the realistic situation.
For my benchmark, I have enough memory, I don't want the CPU waste on GC, I want to use
ballast
to reduce the GC times, I need a big GOGC value. here is the performance after the adjustment.The throughput increased 25% from 45Mgas/s to 55Mgas/s
Before update
After update.
And most of the time, just keeping the GOGC = 100 is a good decision