andygrove opened a new issue, #7344: URL: https://github.com/apache/datafusion-comet/issues/7344
### What is the problem the feature request solves? The default Comet build uses the system allocator (`std::alloc::System`). On Linux that is glibc malloc. The `jemalloc` and `mimalloc` Cargo features exist but are off by default (`default = ["hdfs-opendal", "delta"]` in `native/core/Cargo.toml`), so the released jars use glibc. glibc keeps freed memory in per-thread arenas and returns little of it to the OS. For a long-lived executor running many native plans, resident memory can stay far above what Comet's memory pool or allocation counter reports, and the container is killed. Evidence so far: - #7108: an executor is OOM-killed while Comet's pool and allocation counter stay under the limit, because glibc keeps several GiB of freed native memory. - #4576: pool bookkeeping was accurate to within tens of MB, but the gap between pool reserved and resident memory was 1.5-4 GB per executor, and resident memory did not scale with the configured budget. That run measured jemalloc's `resident`, so it also shows what a jemalloc build holds on to. The comparison against glibc is what is still missing. - #1648: users report running Comet with jemalloc via `LD_PRELOAD` because it returns freed memory to the OS more predictably and has a useful memory profiler. The same thread has a report that mimalloc kept memory allocated and ran into OOMs, and that jemalloc fixed it. It also has a macOS result where mimalloc removed allocator mutex contention, and a Linux TPC-H run that went from about 275 s to 266 s with mimalloc. Two open PRs mitigate the glibc behavior without changing the allocator: #7304 trims the native heap when an executor's last native plan finishes, and #7342 documents `-XX:TrimNativeHeapInterval`. A review comment on #7304 notes that if the release build moved to jemalloc, the glibc arena problem would go away and the trim hook would do nothing, so that decision should be made first. ### Describe the potential solution Decide whether the released native library should be built with jemalloc, and if so, make that the default. This issue is for collecting the data and the objections before anyone opens a PR. Questions to settle: 1. Does jemalloc fix #7108? The reproduction in #7304 is being run with idle `malloc_trim`, `-XX:TrimNativeHeapInterval` and `MALLOC_ARENA_MAX`. The jemalloc arm has not run yet. Peak RSS, pass/fail and total time on the same job would settle most of this. 2. Performance. Compare jemalloc against the system allocator on TPC-H and TPC-DS, on Linux x86_64 and aarch64. Include the cost of the `AccountingAllocator` wrapper, which wraps whichever backend is selected (#6165 measured about 5% on TPC-H Q21 from thread-local lookups). 3. What jemalloc does not cover. The JVM, Arrow Java and Spark's own off-heap allocate through glibc in the same process. In #4576 about 1 GB of an executor's RSS was outside jemalloc, so a jemalloc build narrows the problem but does not remove the need for headroom. 4. Tuning. jemalloc returns memory on a decay timer. How should users set it, and what is the default we ship? With `tikv-jemallocator`, the symbols are prefixed, so the environment variable is probably `_RJEM_MALLOC_CONF` rather than `MALLOC_CONF`. That needs confirming and documenting. 5. Build and portability. The `jemalloc` feature already uses `disable_initial_exec_tls` because the library is loaded by the JVM. Check the behavior when the JVM loads and unloads the library, the minimum glibc we support (#138 notes allocator crates that failed to build on old glibc), macOS, and targets where jemalloc does not build (MSVC already falls back to the system allocator). 6. Interaction with users who already `LD_PRELOAD` jemalloc. Mixing a preloaded allocator with an in-library one needs a clear answer in the docs. 7. mimalloc or jemalloc. #1648 has reports for and against each. Only jemalloc has a measured memory-retention advantage on long-running executors so far. ### Additional context If the data favors jemalloc, the change would likely be to enable the feature in the release build and in `make release`, keep an opt-out, and add a tuning section to the user guide. If it does not, #7304 and #7342 are the fallback, and the result should be written up in the tuning guide. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
