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

Use Jemalloc allocator #3122

Closed
wants to merge 2 commits into from
Closed

Use Jemalloc allocator #3122

wants to merge 2 commits into from

Conversation

mstrug
Copy link
Contributor

@mstrug mstrug commented Nov 12, 2024

Description

Basing on test results (link) of using custom allocators on Staging environment, applied jemalloc allocator as it uses the least memory.

Changes

Removed mimalloc dependency. Added dedicated macro use_custom_global_allocator!() in shared crate, which declares Jemalloc as Rust global allocator. This macro is used all executables.

How to test

Verify Grafana metrics after longer usage of jemalloc to verify if there is no regression comparing to mimalloc.

@mstrug mstrug marked this pull request as ready for review November 12, 2024 22:54
@mstrug mstrug requested a review from a team as a code owner November 12, 2024 22:54
@m-lord-renkse
Copy link
Contributor

Besides staging, was it tested in production for any network? I am mostly curious, traffic isn't the same in staging and prod 🤔 is there a testing plan for this before the final merge?

@mstrug
Copy link
Contributor Author

mstrug commented Nov 13, 2024

Besides staging, was it tested in production for any network? I am mostly curious, traffic isn't the same in staging and prod 🤔 is there a testing plan for this before the final merge?

No, it wasn't tested in our production environment. From Grafana briefly I see the memory usage of Arbitrum One prod network is on similar level as for Staging mainnet.
Also jemalloc library has long development history and is used as FreeBSD default memory allocator and by some services in Facebook, Firefox (according to this site), so from stability point of view it should work fine.
What options regarding testing on prod we have? Should it be firstly deployed to Sepolia, then to Arbitrum/Gnosis and finally Mainnet?

@squadgazzz
Copy link
Contributor

I tested it previously on prod, but before merging it, we should definitely try this again on mainnet prod. Because my tests were under different circumstances. Since I am on-call this week, I can do that once I have time. The behavior on prod is completely different from staging due to a much higher load.

@squadgazzz
Copy link
Contributor

squadgazzz commented Nov 14, 2024

Based on the latest prod run, this allocator is not suggested to be used(at least for the orderbook and driver-liquidity).

upd: make is required to be added to the Docker file build section.

@mstrug
Copy link
Contributor Author

mstrug commented Nov 19, 2024

Closing, as current results on prod suggest regression.

@mstrug mstrug closed this Nov 19, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants