andygrove commented on PR #5934: URL: https://github.com/apache/datafusion-comet/pull/5934#issuecomment-5670790447
Follow-up on the `native_allocated > jemalloc_allocated` samples from the comment above: root cause found, fixed in ce9b13a. **Cause** `AccountingAllocator::dealloc` called `inner.dealloc` first and subtracted afterwards, mirroring `alloc` and `realloc`. jemalloc 5.3 decrements `stats.allocated` at the very start of a large free (`large_dalloc_prep_impl` runs `arena_large_dalloc_stats_update` before `large_dalloc_finish_impl` releases the extent), and for blocks above its 8 MiB oversize threshold the release is eager: the pages go straight back to the kernel. Unmapping 100+ MB of resident memory takes milliseconds. For that whole window jemalloc had already forgotten the block but the balance still carried it, so `native_allocated` read high by exactly the size of the block being freed. The affected samples were all during task teardown, where many large buffers are dropped back to back. **Evidence** - It is not a sampling race. The two counters are logged 1 us apart on the same thread, and the anomalous samples have the same read gap as normal ones. The excess persists at a fixed value across consecutive samples from different threads, then the balance drops by that amount while jemalloc stays flat. - A standalone repro (one thread allocating, touching and freeing a block in a loop, a sampler reading jemalloc then the balance) reproduces it deterministically and the episode length tracks the duration of the free: | ordering | block | samples with native > jemalloc | longest episode | slowest free | |---|---|---|---|---| | subtract after (previous) | 128 MiB | 16.5% | 9.3 ms | 9.3 ms | | subtract after (previous) | 4 MiB | 4.4% | 413 us | 413 us | | subtract before (fixed) | 128 MiB | 0.01% | 15 us | 11 ms | | subtract before (fixed) | 4 MiB | 3.2% | 25 us | 355 us | The residual in the fixed rows is the sampler's own 1 us gap catching an allocation on the other thread, not a lag. **Fix** Subtract before delegating. A free cannot fail, so the reason `alloc` and `realloc` account after the fact does not apply to `dealloc`. With this the balance never includes memory the allocator has already handed back, so the "lower bound" relationship to jemalloc holds at every instant rather than modulo in-flight frees. A new test, `dealloc_settles_before_delegating`, wraps a recording inner allocator and asserts the balance has already dropped by the time the inner free is called. Under the previous ordering it fails with `inner dealloc saw balance 67182807, expected at most 33628375`. **Re-run of the traced TPC-H SF100 suite with the fix** | | before (303875f) | after (ce9b13a) | |---|---|---| | samples with native > jemalloc | 156 of 9269 | 4 of 9272 | | worst excess | 160 MB | 9.4 MB | | median native/jemalloc ratio | 0.90 | 0.90 | The four remaining samples have a 1 to 3 us read gap, consistent with the residual above. Result hashes are unchanged and the traced total is 208.2 s against 207.7 s for the `jemalloc`-only baseline. -- 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]
