andygrove commented on PR #5934: URL: https://github.com/apache/datafusion-comet/pull/5934#issuecomment-5670362800
I ran TPC-H at SF100 against this PR to check the feature end to end and to get a first read on the per-allocation overhead that the description lists as not yet measured. **Setup** - Commit 303875f built twice in separate worktrees: `make release COMET_FEATURES="jemalloc,alloc-accounting"` and, as the baseline, `make release COMET_FEATURES="jemalloc"`. - Single node, Spark 4.1.1 standalone, 2 executors x 8 cores, 16 GB heap + 16 GB off-heap per executor, local Parquet at SF100. Driven by `benchmarks/tpc/run.py`. - One traced run of each build (`spark.comet.tracing.enabled=true`), then three untraced iterations of each build for timing. **Correctness** Result hashes and row counts are identical between the two builds for all 22 queries, in both the traced and untraced runs. No errors, spills, or task retries in either. **Overhead** (untraced, median of 3 iterations per query) | | `jemalloc` | `jemalloc,alloc-accounting` | |---|---|---| | sum of per-query medians | 200.3 s | 203.1 s (+1.4%) | | per-iteration totals | 199.7 / 206.1 / 206.8 s | 202.9 / 208.3 / 209.1 s | The per-iteration ranges overlap. Only Q21 is slower in all three iterations (29.9 s vs 30.9 s, about +3%). Q10 is bimodal (7.5 s or 13 to 14 s) on both builds, so that is unrelated to this change. **What `native_allocated` shows** - Peaks around 2.1 GB per executor and tracks `jemalloc_allocated` at a median ratio of 0.90, which is consistent with jemalloc metadata and size-class rounding. - It does expose the gap the PR is after. At the sample points below the per-thread pool reservations summed to far less than the bytes actually handed out: | query | peak `native_allocated` | pool total at that point | |---|---|---| | Q7 | 1627 MB | 765 MB | | Q8 | 2011 MB | 428 MB | | Q16 | 1192 MB | 0 MB | | Q17 | 1292 MB | 34 MB | - One thing I could not explain: about 2% of sample points (mostly in Q10, Q17 and Q18) report `native_allocated` above `jemalloc_allocated`, by up to 160 MB. Both are logged a few microseconds apart on the same thread, and the un-flushed per-thread drift can only account for a few MB in total, so this is not the 64 KiB batching. It may be jemalloc's stats being approximate under concurrent frees on other threads, but it is worth understanding before treating the value as a strict lower bound relative to jemalloc. Traces and result JSON are available if useful. -- 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]
