-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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 custom Ying memory profiler and prevent giant allocations #5649
Conversation
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. This will disable process level jemalloc profiling in devnet. But depending on the Ying allocator is implemented, jemalloc profiling is probably still enabled in private testnet which would be fine.
@@ -51,9 +51,5 @@ COPY --from=builder /sui/target/release/sui-node /usr/local/bin | |||
|
|||
ARG BUILD_DATE | |||
ARG GIT_REVISION | |||
# Use jemalloc as memory allocator | |||
ENV LD_PRELOAD /usr/lib/x86_64-linux-gnu/libjemalloc.so |
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.
Is this removed to get clearer signal, or you have found incompatibilities? In the longer term we probably want to use jemalloc regardless of its profiling feature.
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.
Yeah this is just so that we can do it environment by environment, to be safer
0275e24
to
594b5f4
Compare
This change swaps out the explicit Jemalloc-based memory profiling code with my custom Ying profiler (https://github.com/velvia/ying-profiler), using the GlobalAlloc trait. Why?
I have run local stress benchmark with the profiler turned on, and the QPS remained about the same as before (roughly ~360 when target = 400).
NOTE: the large memory allocation prevention will be updated in this PR shortly by uopdating the git pointer
This does not prevent one from using jemalloc, which can still be used by adding
LD_PRELOAD
and setting MALLOC_CONF etc. It does switch the default to the default Rust memory allocator underneath though.