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

Add callback for the soft memory limit handling #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abyss7
Copy link

@abyss7 abyss7 commented Nov 13, 2024

This feature is useful i.e. to handle large heap allocations, pre-OOM state, or just signal about a certain size of heap.

@ckennelly
Copy link
Collaborator

Thanks for the pull request.

It's somewhat tricky to support a callback right here, since we're holding pageheap_lock. We read stats about memory usage under that lock, and that plausibly seems like something a zero-argument callee might need to do.

For our internal use cases that need to track memory usage, we've had a lot of success with using callbacks on our sampled allocations: It allows the application to track memory usage with about 2MB of error either way and can be done without holding any locks. While we haven't (yet) open-sourced these APIs, is that something that would work for your use case?

@abyss7
Copy link
Author

abyss7 commented Nov 29, 2024

While we haven't (yet) open-sourced these APIs, is that something that would work for your use case?

Sounds like a good solution, if these APIs get open-sourced.

This pull request emerged from our internal code - we don't collect stats immediately, but fork the process in another thread, stop the parent, and dump any stats inside the child - relying on tcmalloc's heap state shared between processes.

We have another patch to make tcmalloc more fork-friendly to supplement our technique - I'm not sure if it was also open-sourced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants