andygrove commented on issue #5383:
URL: 
https://github.com/apache/datafusion-comet/issues/5383#issuecomment-5319082686

   I ran a single case from `CometShuffleBenchmark` with the counters from 
#5384 enabled, and the traffic turns out to come from exactly one consumer, 
with a much more specific shape than I assumed when I filed this.
   
   ## Setup
   
   `SQL Single INT Shuffle(201 Partition)` over 10M rows, native shuffle, 
release build, `spark.memory.offHeap.enabled=true` with 4 GB off-heap, default 
`fair_unified` pool, `local[5]`. Stats collected by raising only 
`comet::execution::memory_pools` to debug via a custom `log4rs.yaml`.
   
   ## Measured traffic
   
   110 tasks (11 iterations x 10 tasks), all identical:
   
   ```
   Task 4334 CometFairMemoryPool memory pool stats: acquire(calls=256, 
requested=25165824 bytes,
     granted=25165824 bytes, short=0), release(calls=1, bytes=25165824), 
backend_time=432.836µs
   ```
   
   | | |
   |---|---|
   | Total calls | 28,270 (28,160 acquire, 110 release) |
   | Total time inside the round-trip | 36.0 ms |
   | Avg per call | 1.27 µs (p50 1.23, p90 1.39, max 1.86) |
   | Per task | 0.32 ms (p50), 0.51 ms (max) |
   | Short grants | 0 |
   
   Per-task wall time is roughly 182 ms x 5 slots / 10 tasks = 91 ms, so the 
round-trips are about **0.35% of a shuffle-write task**, uncontended. Two runs 
of the same configuration produced identical call counts and the same 1.27 µs, 
so the measurement is stable.
   
   ## The consumer
   
   With `spark.comet.debug.memory` enabled (note it has to be set through 
SQLConf so it reaches native via the serialized Comet confs -- a system 
property does not get there), there is exactly one registrant:
   
   ```
   28160 [ShuffleRepartitioner[0]].try_grow
     110 [ShuffleRepartitioner[0]].shrink
     110 [ShuffleRepartitioner[0]].register / unregister
   ```
   
   Nothing else touches the pool. The Comet-with-Spark-shuffle and 
Comet-JVM-shuffle cases in the same benchmark make **zero** memory pool calls.
   
   Sizes per task:
   
   | Size | Calls/task | What it is |
   |---|---|---|
   | 32,768 | 247 | **96% of all calls** -- one 8192-row Int32 Arrow data 
buffer (8192 x 4) |
   | 98,304 | 2 | batches pinning more buffers |
   | 163,840 ... 8,421,376 | 7, one each | a doubling series: 160 KB, 288 KB, 
544 KB, 1,056 KB, 2,080 KB, 4,128 KB, 8,224 KB |
   | 25,165,824 (shrink) | 1 | the entire 24 MB released in one call |
   
   The site is `native/shuffle/src/partitioners/multi_partition.rs:463`: 
`buffer_partitioned_batch_may_spill` performs exactly one 
`try_grow(mem_growth)` per input batch, where `mem_growth` is 
`count_new_buffers(...)` -- the 32 KB steps -- plus the growth of the 
per-partition index vectors, which is the doubling series as the 201 partition 
vectors reallocate. The single `shrink` is the `reservation.free()` at `:528`.
   
   ## What this changes about the plan above
   
   The release side is already effectively batched (one call per task), so 
hysteresis buys nothing on this workload; the entire cost is on the acquire 
side, and it is one consumer growing per batch rather than many consumers 
churning.
   
   That makes **option E the sharper tool than option A here**: if the 
repartitioner rounded its reservation up to a step of a megabyte or so and 
tracked exact usage locally, 256 calls per task would become a handful. It 
needs no new config, no retained slack, and therefore does not pull in option 
B. Option A would get the same result for this workload but pays with slack 
that Spark cannot reclaim.
   
   So I would reorder the sequencing to: instrumentation (#5384) -> C -> E -> A 
and B only if other workloads show many small consumers rather than one 
batch-driven one.
   
   Caveat on the ceiling: 0.35% is a shuffle-write task at 5 concurrent tasks 
with no lock contention. I have not measured aggregate/sort/join-heavy plans, 
which have several consumers per task and may look quite different.
   


-- 
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]

Reply via email to