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

   Retracting part of my previous comment. I said the kernel corroborates the 
resident finding, citing `/proc` VmRSS of about 5.2 GB per executor. That was 
wrong, and the RSS argument does not hold.
   
   ## What I did
   
   I added a kernel `process_rss` metric (`/proc/self/status` VmRSS) and ran 
TPC-H SF100 on a **default allocator** build 
(`COMET_FEATURES=alloc-accounting`, no jemalloc), since a jemalloc-only signal 
says nothing about what most people run. Executor RSS came out at p50 6.7 to 
7.1 GB, max 7.8 to 8.6 GB.
   
   I then ran the control I should have run first: the same benchmark, same 
memory configuration, with Comet not loaded at all.
   
   | | RSS p50 | RSS max |
   | --- | --- | --- |
   | Comet, default allocator | 6.7 to 7.1 GB | 7.8 to 8.6 GB |
   | Spark only, no Comet (`spark.plugins = None`) | **9.28 GB** | **17.33 GB** 
|
   
   Both arms used `spark.executor.memory=16g`, 
`spark.memory.offHeap.enabled=true`, `spark.memory.offHeap.size=16g`, the same 
queries, and the same external `/proc` sampler.
   
   The baseline without Comet is **higher** than the Comet run. VmRSS is 
dominated by the JVM heap (`-Xmx16384M`), so an absolute RSS number cannot be 
attributed to Comet, and no positive delta exists here to attribute.
   
   Caveat on the comparison: Spark only took 508.5 s against Comet's roughly 
210 s, so the two arms did different work over different durations, and heap 
occupancy depends on GC timing. It is a loose comparison. It is decisive in the 
one direction that matters, which is that it cannot support a claim of a Comet 
RSS overshoot.
   
   ## What still stands
   
   - **jemalloc `resident` runs 6x to 8x `allocated`** on the jemalloc build. 
That is Comet's native allocator, unaffected by the JVM heap, and I still 
believe it is real. It is specific to a build that is not the default, since 
`native/core/src/lib.rs` selects the system allocator unless jemalloc or 
mimalloc is requested.
   - **Pool accounting is accurate.** Reserved to native is 15 to 23 MB at p50 
across four runs at two budgets.
   - **Gating reservations does not prevent an overshoot**, as reported earlier.
   
   ## What this changes for this issue
   
   I no longer have any measurement showing that Comet's own contribution to 
resident memory is large. My earlier suggestion that a cgroup or RSS based 
circuit breaker is the thing worth building was based on the RSS reading I am 
now retracting, so it is not currently supported by evidence. The allocator 
agnostic signal is still the right design *if* such a guard is ever built, for 
the reasons given before, but nothing I have measured establishes that it needs 
to be built.
   
   If someone has a workload where Comet executors are killed for exceeding 
container memory, the useful next step is this same paired measurement on that 
workload: RSS with and without Comet at identical configuration. That 
difference is the only number that means anything, and it is cheap to collect.
   
   Apologies for the churn on this issue. The pattern in my three comments is 
that I twice reasoned forward from a large number before establishing what it 
was composed of. The control run should have come first.
   


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