-
Notifications
You must be signed in to change notification settings - Fork 664
Use jemalloc in standalone and add stats. #2470
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
Conversation
|
I suspect we should probably figure out if there are any platforms that this won't work on, since we just went through the work to expand our platform support (#2415). I guess alternatively we could wait to see if any users complain.. |
|
this broadly looks good to me, but I'm not very familiar with different allocator behavior or with the stats part of our code, so I wonder if we can get someone to review who's more confident. |
|
Could you please run subscription benchmarks before/after as well as the update 10^6 benchmark before/after? |
|
benchmarks please |
Criterion benchmark resultsError when comparing benchmarks: Couldn't find AWS credentials in environment, credentials file, or IAM role. Caused by: |
|
Callgrind benchmark in progress... |
bfops
left a comment
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.
Looks good as far as I can tell. We think this will almost certainly be an improvement over the system allocator because jemalloc usually is. And @jsdt says he tested it.
Description of Changes
This uses jemalloc in standalone instead of mimalloc, unless the target env is msvc. The main reason is that jemalloc exposes better stats, and has a way to do heap profiling. This PR doesn't set up profiling, but it does add stats on jemalloc's memory usage.
Expected complexity level and risk
Testing
This works on mac and at least some linux platforms.