Use jemalloc on non-windows to reduce memory fragmentation #738
Conversation
There was a problem hiding this comment.
fyi: @chrisstaite-menlo
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, publish-image, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @aaronmondal)
To help reduce/remove memory fragmentation caused by memory store we are switching our allocator to jemalloc. This should make our memory compaction significantly more efficient. closes TraceMachina#621
There was a problem hiding this comment.
It's great that we have solved this here and I think it's a fantastic ongoing experiment for us. The memory_storewill continue to surface bleeding edge issues.
Now, more than ever I think we need to land the redis_store (not actually vendor specific) functionality. cc @blakehatch and @zbirenbaum
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, publish-image, ubuntu-20.04 / stable, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @aaronmondal)
Agreed I'll be dusting off the old redis store PR soon with a fresh rebase and clean up how the mocking is done. Will need to set up some integration tests with @aaronmondal and would love @zbirenbaum's help as well! |
There was a problem hiding this comment.
We should try and collect some previous metrics on memory consumption and deploy this collecting same metrics to see a difference. jemalloc does allow for easier profiling and is a battle harden allocator, ymmv depending on what this is to solve. Longer term we should probably have some like https://www.polarsignals.com/blog/posts/2023/12/20/rust-memory-profiling integrated to pull a profile while system is running.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), publish-image (waiting on @aaronmondal)
There was a problem hiding this comment.
I did run a bunch of local experiments on this and found that a memory store configured to cache 500M would use up to ~3-5G of data using glibc and never come down. Using jemalloc the same config would reach around the same top, but then once I stopped hammering the CAS to cause this issue, I would then let it sit idle for a few mins and it'd settle down to ~600M.
I considered alternative methods, like using a custom allocator for the data stored in the memory store, but because we are using the Bytes library, we are forced to use the global allocator. We could still get around this issue by forcing every read of the memory store CAS to make a copy of the data when sending it out over the wire; right now we don't need to make any copies for read operations.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), publish-image (waiting on @aaronmondal)
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), publish-image (waiting on @aaronmondal)
Cargo.toml line 57 at r1 (raw file):
[target.'cfg(not(target_env = "msvc"))'.dependencies] tikv-jemallocator = "0.5.4"
It looks like we'd have to patch (or rewrite) the buildscript for the crate. The previous attempt to use MiMalloc #736 passed tests etc but didn't solve the defragmentation issue.
If I understand microsoft/mimalloc#632 correctly it seems like it's possible to pull the defragmentation functionality from jemalloc into mimalloc with a patch like this: https://github.com/romange/helio/tree/master/patches
Maybe it's worth a shot to attempt using such a patched version of mimalloc rather than trying to fix the buildscript for tikv-jemallocator?
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), publish-image (waiting on @aaronmondal)
Cargo.toml line 57 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
It looks like we'd have to patch (or rewrite) the buildscript for the crate. The previous attempt to use MiMalloc #736 passed tests etc but didn't solve the defragmentation issue.
If I understand microsoft/mimalloc#632 correctly it seems like it's possible to pull the defragmentation functionality from jemalloc into mimalloc with a patch like this: https://github.com/romange/helio/tree/master/patches
Maybe it's worth a shot to attempt using such a patched version of
mimallocrather than trying to fix the buildscript fortikv-jemallocator?
Hmmm, maybe we should just make the copy in a different allocator then :-( .
To help reduce/remove memory fragmentation caused by memory store we are switching our allocator to jemalloc. This should make our memory compaction significantly more efficient.
closes #621
This change is