docs: separate physical memory regions from accounting authorities in the container diagram

[andygrove]

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. The subgraph backgrounds are left to the mermaid theme so the fence stays legible in GitHub's dark mode as well as light.

%%{init: {'flowchart': {'wrappingWidth': 420}}}%%
flowchart LR
  subgraph POD["Executor container"]
    direction TB

    subgraph NAT["Native heap"]
      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"]
      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:#f6c69a,stroke:#8a4b1f,color:#000
  class NRES,TUNG,JSH,OFFPOOL acct
  class SOBJ,COBJ,ONPOOL heapacct
  class NUND,ARW,NONHEAP,MISC,NOONE unacct

Loading

_{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.}

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/mermaid-cli@11.17.0 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.

[comphead]

Checked the redraw against the sources. Separating region from authority is the right axis and worth landing. Four things I would change first.

1. The reservation-ceiling label is false for one pool type and imprecise for the other.

parse_memory_pool_config gives greedy_unified a pool_size of 0 (native/core/src/execution/memory_pools/config.rs:57-62) and CometUnifiedMemoryPool never reads a limit, so spark.comet.exec.memoryPool.fraction has no effect there at all. Under the default fair_unified the check is pool_size / num_consumers against the pool's total used, not the requester's own (fair_pool.rs:149-163), so the ceiling is a fraction of a fraction.

Neither is usually the binding constraint. ExecutionMemoryPool.acquireMemory caps a task at maxPoolSize / numActiveTasks and parks it below poolSize / (2 * numActiveTasks). With 8 concurrent tasks a Comet plan gets roughly 1/8 of the off-heap pool, not all of it.

2. COBJ --> ONPOOL is a new inaccuracy. The unified memory manager accounts only for what consumers explicitly reserve plus cached block sizes. Comet's plans, CometVectors and iterators are ordinary heap objects in user memory, the 1 - spark.memory.fraction slice. The old undifferentiated HEAP box was vaguer but less wrong.

3. The single OFFPOOL box hides the execution/storage split. spark.memory.offHeap.size is divided by spark.memory.storageFraction into off-heap execution and off-heap storage pools (UnifiedMemoryManager.scala:64,98-102). Comet only touches the execution half, and reaches the rest only by evicting OFF_HEAP cached blocks.

4. Scope is missing. The pool, the reservation and the fair share are all per task attempt (task_shared.rs), but the diagram is drawn per container, so a reader cannot see that spark.executor.cores / spark.task.cpus of these run at once.

Two things that predate this PR but sit next to the diagram:

• memory_management.md still says to compare jemalloc_allocated against "the summed thread_NNN_comet_memory_reserved values". Since feat: add native allocation accounting for memory observability #5934 there is an alloc-accounting feature emitting native_allocated (native/core/src/alloc_accounting.rs), and jni_api.rs:252-255 warns that the per-thread counters must not be summed, because a shared pool reports its full reservation on every thread that references it. comet_memory_reserved_total is the figure to use. tracing.md has this right, and the "No signal for real native usage" open problem is stale for the same reason.
• Each createPlan builds its own RuntimeEnv (jni_api.rs:618,793) and therefore its own FileMetadataCache, capped at DataFusion's 50 MiB default. A shuffle runs two plans per task, so eight concurrent tasks can hold up to roughly 800 MiB of Parquet metadata that no pool sees. Worth a node.

An alternative diagram covering all of the above is in the next comment.

[comphead]

An alternative to the diagram in this PR, covering the four points above.

%%{init: {'flowchart': {'wrappingWidth': 320}}}%%
flowchart LR
  subgraph CG["Executor container, cgroup memory.max"]
    direction TB

    subgraph NAT["Native heap, Rust"]
      direction TB
      NRES["Declared reservations<br>sort, aggregate, joins, shuffle writer"]
      NUND["Undeclared<br>kernels, builders, Parquet buffers, FileMetadataCache, tokio, FFI batches"]
      NOPAQ["Outside GlobalAlloc<br>libzstd, aws-lc-sys, libhdfs, fragmentation, padding"]
    end

    subgraph OFF["JVM off-heap, Unsafe and Java Arrow"]
      direction TB
      TUNG["Spark Tungsten pages"]
      JSH["Comet shuffle pages"]
      ARW["CometArrowAllocator, no limit"]
    end

    subgraph HEAP["JVM heap, spark.executor.memory"]
      direction TB
      UNIF["Unified region, spark.memory.fraction 0.6"]
      USER["User memory, the other 0.4<br>Comet plans and vectors, bounded only by -Xmx"]
    end

    subgraph REST["Neither heap"]
      direction TB
      NONHEAP["JVM non-heap<br>metaspace, code cache, stacks, Netty"]
      PAGEC["Page cache from spill and shuffle files"]
    end
  end

  subgraph GATE["Admission gate"]
    direction TB
    G1["Comet, fair_unified only<br>memory_limit / num_consumers"]
    G2["Spark, per task attempt<br>pool / numActiveTasks"]
    G0["No gate"]
  end

  subgraph BUD["Budget"]
    direction TB
    OFFEX["Off-heap execution pool, per executor"]
    OFFST["Off-heap storage pool, storageFraction"]
    ONPOOL["On-heap execution and storage pools"]
    NOONE["No budget<br>memoryOverhead is the only slack"]
  end

  NRES -->|"try_grow over JNI"| G1
  G1 --> G2
  TUNG --> G2
  JSH --> G2
  UNIF -->|"on-heap mode"| G2
  G2 --> OFFEX
  G2 --> ONPOOL
  OFFEX <-->|"borrow, evict"| OFFST
  NUND --> G0
  NOPAQ --> G0
  ARW --> G0
  USER --> G0
  NONHEAP --> G0
  PAGEC --> G0
  G0 --> NOONE

  classDef budgeted fill:#c7ecd0,stroke:#2f6b46,color:#000
  classDef gate fill:#fff2b2,stroke:#8a7420,color:#000
  classDef unbudgeted fill:#f6c69a,stroke:#8a4b1f,color:#000
  class NRES,TUNG,JSH,UNIF,OFFEX,OFFST,ONPOOL budgeted
  class G1,G2 gate
  class NUND,NOPAQ,ARW,USER,NONHEAP,PAGEC,G0,NOONE unbudgeted

Loading

Three columns, and every byte takes the same shape of path: a physical region, the gate that admits it, the budget it draws down. Making "no gate" an explicit node is what keeps the arrows aligned, and it also states the thing the current diagram can only imply.

What it adds over the version in the PR:

• The fraction and the 1/numActiveTasks divisor sit in the gate column, where they apply, instead of in a region label.
• The off-heap pool is two boxes with a borrow edge, so OFF_HEAP cached blocks are visibly competing for the same bytes.
• The heap separates the unified region from user memory, which is where Comet's own JVM objects actually live.
• FileMetadataCache appears, at 50 MiB per plan.

Two things deliberately left to prose rather than drawn. The native heap splits three ways by which counter can see it: reservations show in the pool and in native_allocated, undeclared allocations only in native_allocated, and the GlobalAlloc bypasses in neither. And the Comet-to-Spark edge is one way, because NativeMemoryConsumer.spill() returns 0.

If that is still too much for one figure, the natural split is to move the gate and budget columns into a separate "what a single reservation passes through" diagram under Where Comet's budget comes from, and leave this one region-to-authority only.

I have not rendered the fence. dev/ci/check-mermaid.py is the authority and it does not run on my machine either.