andygrove commented on issue #6050:
URL:
https://github.com/apache/datafusion-comet/issues/6050#issuecomment-5744393923
Some history on how this came about, since it turns out no single change
decided that off-heap mode needs no overhead.
#1379 introduced the off-heap distinction. In off-heap mode the plugin added
`getCometShuffleMemorySize` rather than the full Comet overhead, and
`shouldOverrideMemoryConf` gained `&& (!cometUnifiedMemoryManagerEnabled(conf)
|| !cometShuffleUnifiedMemoryManagerInTestEnabled(conf))`, so off-heap mode
still got something.
#1485 then removed the
`COMET_COLUMNAR_SHUFFLE_UNIFIED_MEMORY_ALLOCATOR_IN_TEST` config and collapsed
that predicate to a plain `&& !unifiedMemory`. That is the point where off-heap
mode stopped getting any overhead at all, and it happened as a side effect of
deleting a test-only config rather than as a deliberate decision — the off-heap
branch in the plugin became dead code.
#1525 deleted that dead branch and replaced it with
`assert(!isOffHeapEnabled(sc.getConf))`, and #1561 produced the shape that is
in `main` today: the early `return 0` in `getCometMemoryOverheadInMiB` with the
comment "when running in off-heap mode we use unified memory management to
share off-heap memory with Spark so do not add overhead", plus the
`logInfo("Comet is running in unified memory mode and sharing off-heap memory
with Spark")` else-branch. #2599 later replaced the factor/minimum calculation
with the flat `spark.comet.memoryOverhead` but left the off-heap `return 0`
alone.
So the comment in the code reads like a considered position, but it is
really a description of where the refactoring landed. The premise behind it,
that unified memory management means Comet needs no additional container
headroom, only holds for the memory that operators actually reserve.
--
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]
