andygrove opened a new pull request, #6121:
URL: https://github.com/apache/datafusion-comet/pull/6121
## Which issue does this PR close?
N/A — no issue; this is a documentation-only redraw.
## Rationale for this change
The "What the container sees" diagram in the memory management guide groups
consumers by whether Spark can see them. That forces one box per (region,
accountant) pair, and the one relationship it cannot express is the one that
makes Comet's memory model confusing: **a native reservation is charged against
`spark.memory.offHeap.size` while occupying native heap.** Physical location
and accounting authority are independent axes, and a diagram that collapses
them into one encourages the reading that native memory is budgeted by
`spark.executor.memoryOverhead` — which it is not. `memoryOverhead` is a
container-sizing term, not a Comet budget; Comet's ceiling is
`spark.memory.offHeap.size * spark.comet.exec.memoryPool.fraction`.
## What changes are included in this PR?
Redraws the diagram with physical regions on the left, budget authorities on
the right, and arrows between them, so a byte's region and its budget can
differ and you can see it.
Also picks up what the old diagram omitted:
- the JVM heap split into Spark and Comet objects,
- C libraries that allocate outside Rust's global allocator (libzstd,
libhdfs, `aws-lc-sys`), which the "Accounting gap" section already discusses,
- a note that Spark's off-heap pool has no arrow back into the native heap,
because `NativeMemoryConsumer.spill()` returns `0`.
Colour is redundant with the arrow target, so the diagram survives greyscale
printing and colour-blind readers.
```mermaid
flowchart LR
subgraph POD["Executor container: cgroup memory.max = executor.memory +
memoryOverhead + offHeap.size"]
direction TB
subgraph NAT["Native heap: allocated by Rust, no JVM allocator involved"]
direction TB
NRES["Declared operator reservations<br>sort, grouped aggregate,
joins, shuffle writer<br>ceiling = spark.comet.exec.memoryPool.fraction of
spark.memory.offHeap.size"]
NUND["Everything else Rust allocates<br>expression kernels, array
builders, decompression,<br>Parquet and object_store metadata, tokio,<br>C
libraries outside Rust's global allocator"]
end
subgraph OFF["JVM off-heap: allocated by Unsafe and by Java Arrow"]
direction TB
TUNG["Spark Tungsten pages"]
JSH["Comet JVM shuffle pages<br>CometUnifiedShuffleMemoryAllocator"]
ARW["Comet JVM Arrow buffers<br>CometArrowAllocator: a RootAllocator
with no limit"]
end
subgraph HEAP["JVM heap"]
direction TB
SOBJ["Spark objects"]
COBJ["Comet objects: plans, vectors, iterators"]
end
subgraph REST["Rest of the process"]
direction TB
NONHEAP["JVM non-heap<br>metaspace, code cache, thread stacks, Netty"]
MISC["Page cache from spill files<br>fragmentation, padding, jemalloc
retained pages"]
end
end
OFFPOOL["Spark off-heap execution pool<br>TaskMemoryManager,
spark.memory.offHeap.size<br>inside the container limit, not headroom on top of
it"]
ONPOOL["JVM heap accounting<br>unified memory manager,
spark.executor.memory"]
NOONE["Accounted by nobody<br>no budget, no
backpressure;<br>spark.executor.memoryOverhead is the only slack"]
NRES -->|"charged over JNI by CometTaskMemoryManager"| OFFPOOL
TUNG --> OFFPOOL
JSH --> OFFPOOL
SOBJ --> ONPOOL
COBJ --> ONPOOL
NUND --> NOONE
ARW --> NOONE
NONHEAP --> NOONE
MISC --> NOONE
classDef acct fill:#c7ecd0,stroke:#2f6b46,color:#000
classDef heapacct fill:#cfe4fb,stroke:#2f5680,color:#000
classDef unacct fill:#fbe9a8,stroke:#8a6d1f,color:#000
class NRES,TUNG,JSH,OFFPOOL acct
class SOBJ,COBJ,ONPOOL heapacct
class NUND,ARW,NONHEAP,MISC,NOONE unacct
style POD fill:#ffffff,stroke:#333,stroke-width:2px
style NAT fill:#f6f6f6,stroke:#999
style OFF fill:#f6f6f6,stroke:#999
style HEAP fill:#f6f6f6,stroke:#999
style REST fill:#f6f6f6,stroke:#999
```
<sub>The fence above is the diagram as committed, rendered by GitHub. An SVG
pasted into a body is not rendered — GitHub's image proxy will not serve
`image/svg+xml` — so this is the closest thing to the published image.</sub>
## How are these changes tested?
Documentation only, no code paths touched.
The diagram was extracted from the committed markdown and rendered with the
pinned `@mermaid-js/[email protected]` and `docs/puppeteer-config.json`, the
same pair `docs/source/conf.py` gives the site build, so it draws rather than
publishing as a diagram-shaped hole (#6062). `dev/ci/check-mermaid.py` in
preflight is the authoritative check; it cannot run on this macOS box, where
Chrome times out launching under the script's piped stdio and all three fences
fail identically, including the two this PR does not touch.
`npx prettier docs/source/contributor-guide/memory_management.md --check`
passes.
--
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]
