vidya/design-plans/rendergraph-transient-aliasing.md
janis 5e4bc0ccf0
rendergraph: correct transient-resource aliasing with pooled placement
Lifetimes derive from the real dependency ordering (retained DAG +
compile-time happens-before closure), not command-index intervals.
Aliasing is restricted to the single-queue contract; alias reuse is
synchronized with memory/image barriers anchored at first-use commands.

- renderer_device: aliasable shared allocations (CAN_ALIAS), memory-
  requirements queries incl. dedicated hints, offset-aware
  bind_buffer/image_memory2, grouped bind under one allocator lock
- renderer_resources: unallocated resource creation, requirements
  cached in Allocation::Unmanaged (OnceLock), MemoryGroup pooling with
  intersecting memoryTypeBits, bind-time validation
- renderer_rendergraph: retained dependency DAG, happens_before
  closure, batch lifetimes, Placement type, plan_placements with
  pooled aliasing (first-fit over offset-sorted ranges, pool classes
  split by bufferImageGranularity), alias-barrier construction,
  placement/bind error variants
- perf: quadratic compile/resolve paths eliminated (per-resource
  indexing, frontier caching, backward-scan prune, cached recording
  index, recorder scratch reuse); single requirements query per
  resource through plan/validate/bind
- design plan + GPU-gated validation tests
2026-10-27 01:57:27 +01:00

203 lines
13 KiB
Markdown

# Plan: Correct Transient-Resource Aliasing in `renderer_rendergraph`
Status: proposed — rev 3 (code-verified; rev-2 review + follow-up comments applied)
Date: 2026-10-26
Scope: `crates/renderer_rendergraph`, `crates/renderer_resources`, `crates/renderer_device`
## Goal
Fix the broken transient-resource aliasing in the rendergraph: lifetimes must be
derived from the graph's real ordering (dependency DAG + batches, not command
indices), alias reuse must be synchronized with barriers, and the resources
crate must gain the infrastructure to compute correct memory requirements so
correctly sized pools can be allocated and bound to graph-created resources.
## Diagnosis (verified against code)
| # | Defect | Evidence |
|---|--------|----------|
| 1 | Lifetimes are command-index intervals (`first_command`/`last_command`), but commands within a `topo_batches` batch are conceptually concurrent — two resources used by different commands of the same batch are wrongly treated as non-overlapping | `graph.rs:66-79, 862-877` |
| 2 | Batch index is **not** a happens-before proof: the DAG only contains edges for conflicting same-resource accesses and is discarded after `topo_batches`; e.g. test `independent_commands_share_a_batch...` produces batches `[0,2],[1]` with no edge `2→1` | `graph.rs:643-657, 683-697, 699-714` |
| 3 | Queue transfers unmodeled: `QueueTransfer` exists but every barrier sets `None`; no command→queue assignment exists anywhere (`derive_queue_capabilities` is a resource-wide stage union, not a per-command queue) | `graph.rs:41-58, 792, 822, 1461-1541` |
| 4 | No alias-safety barriers: `build_barriers` only tracks prior uses of the *same* resource; nothing orders last use of alias A before first use of alias B sharing memory | `graph.rs:728-835` |
| 5 | Packing uses `texture_size_estimate`/`desc.size`, ignoring real `VkMemoryRequirements`, alignment, memory-type bits, `bufferImageGranularity`, and dedicated-allocation requirements; offsets are never bound to resources | `graph.rs:888-919, 1543-1588` |
| 6 | No offset-aware bind path: `bind_buffer_memory`/`bind_image_memory` bind whole VMA allocations; `Allocation::Shared(Arc<DeviceObject<GpuAllocation>>)` exists but is unused for pooling | `device.rs:209-214, 766-778` |
## Design decisions (post-review)
### D1 — Happens-before, not batch numbers
Retain the dependency adjacency from `GraphCompiler::dependencies()` in
`CompiledGraph` and add an explicit `happens_before(a, b)` reachability query.
Alias A→B is permitted only if *every* terminal use of A happens-before *every*
initial use of B. Batch intervals are computed (for reporting/limits) but never
used as the safety criterion. Same-batch uses always interfere.
`happens_before` is computed as a **transitive closure at compile time**: one
bitset per command (`Vec<FixedBitSet>`-style reachability rows, or bitset
columns if that packs better), built once in `compile()` from the Kahn
adjacency. Queries are then O(1) and deterministic. Rationale: the command
count is bounded by the recorded command list (thousands at most), closure is
O(V·E/64) once, and per-query BFS would be re-run per alias-candidate pair
during placement for no benefit. Note the closure's semantics: edges encode
only conflicting same-resource accesses, so reachability means exactly
"provably ordered via a resource-hazard chain" — precisely the aliasing-safety
criterion. Commands mutually unreachable (including same-batch pairs) interfere.
### D2 — Phase split
`compile()` stays device-free: dependency order, batch lifetimes, interference.
A new device-dependent `plan_placements(device)` (called from `resolve`) does
requirements queries, pool placement, alias-barrier construction, and binding.
Tests that call `bind_lifetimes()` directly are updated.
### D3 — Conservative queue policy
There is no command→queue assignment today and all commands record into one
command buffer. Aliasing is restricted to the single-queue case; any pair whose
accesses can't be proven ordered on one queue interferes. Cross-queue aliasing
(with ownership transfers/semaphores) is explicitly out of scope and documented
as a follow-up — no fake `QueueTransfer` support.
### D4 — Alias synchronization done correctly
- For aliased **buffers**: emit a `VkMemoryBarrier2` (new `MemoryBarrier` +
`PipelineBarrier.memory_barriers`) before the new occupant's *first-use
command* (a concrete command, not "batch start"), with `src` = old occupant's
aggregated terminal stage/access (writes; execution-only if read-only),
`dst` = new occupant's aggregated first-use scope.
- For aliased **images**: fold the old occupant's terminal scope into the new
image's first-use image barrier (`srcStageMask`/`srcAccessMask` = old scope,
`oldLayout = UNDEFINED`) — a bare memory barrier does not chain with a
following `srcStageMask=NONE` layout transition.
- Aggregate multi-scope accesses per command (OR all stage/access for the
resource at that command).
- Placement is range-based: a new resource's `[offset, offset+size)` is tested
against **all** occupied ranges; barriers include every old occupant
intersecting the range.
### D5 — Vulkan aliasing correctness details
- `bufferImageGranularity`: **separate pools per class** (linear-only and
optimal-only) is the primary strategy — first-fit stays simple and no
padding logic is needed. Intra-pool offsets are rounded only to each
resource's `alignment`. (Intra-pool interleaving with granularity-driven
offset rounding is a deliberate non-goal; revisit only if pool-class split
waste ever shows up in practice.)
- Set `VK_IMAGE_CREATE_ALIAS_BIT` on pooled images.
- Exclude from pools: `requiresDedicatedAllocation` resources, explicit
`AllocationStrategy::Dedicated` (incl. the 64 MiB heuristic), and
host-visible (`CpuToGpu`/`GpuToCpu`) resources — `Buffer::map()` returns
slices from the allocation base and would silently map wrong bytes unless
suballocation offsets are tracked.
- Created-but-unused resources: skip creation entirely (or allocate outside the
pool) — decided as "skip unused transients", with a test.
- Pooled images must have `GraphTextureDesc.layout == UNDEFINED`:
`plan_placements` rejects non-UNDEFINED declared initial layouts on pooled
images with a graph error (a created image's declared initial layout cannot
be honored by reused memory, whose contents are always undefined at first
use). Non-pooled images keep today's behavior.
### D6 — Device/resources infra (new APIs, not existing ones)
- `renderer_device`: `allocate_memory(requirements, …)` via
`vk_mem::Alloc::allocate_memory` with `CAN_ALIAS`; `find_memory_type_index`;
`bind_buffer_at_offset`/`bind_image_at_offset` via `bind_*_memory2` with
`allocation_local_offset`; require API ≥1.1 or enable the full
`get_memory_requirements2`/`bind_memory2`/`dedicated_allocation` extension
set with a clear error otherwise (same guard pattern as the existing
dedicated-allocation check in `device.rs`); define shared-allocation
ownership/cleanup: at bind time the group **transfers** the
`Arc<DeviceObject<GpuAllocation>>` into each bound resource's
`Allocation::Shared` and drops its own reference — the underlying memory is
freed when the last bound resource drops (via `DeviceObject` drop), matching
the existing `Owned`/`Shared` semantics. The group never retains a post-bind
handle.
- `renderer_resources`: symmetric `Buffer::new_unallocated` /
`Image::new_unallocated`, `memory_requirements()` (incl.
`VkMemoryDedicatedRequirements`), `bind_allocation(alloc, offset)` with
alignment/type-bits/bounds validation; a `memory` module with
`MemoryRequirements` + `MemoryGroup` (bind → `Allocation::Shared`). Pool
groups are keyed by a memory type that is in **every member's**
`memoryTypeBits` (intersection), not by any single member's individually
best type; a member whose bits exclude the group's type moves to another
group or its own allocation. New `renderer_resources::Error` variants for
failed requirement queries / bind-time validation (alignment, type-bits,
bounds); `renderer_rendergraph` maps these into new `GraphError` variants
(e.g. `PlacementFailed(ResourceId)`, `BindFailed(ResourceId)`) — bind/placement
failures surface through `resolve`/`plan_placements`'s existing
`renderer_resources::Result` boundary and map at the call site. No graph
types leak into this crate.
## Task breakdown (ordered, dependency-explicit)
1. **`renderer_device`**: memory-type query, aliasable shared allocation,
offset-aware bind, API/extension guards, error variants, ownership docs.
Tests with `TestVulkan` (`--features testing`).
2. **`renderer_resources`**: unbound create + requirements + bind APIs for
buffer *and* image; `memory` module (`MemoryRequirements`, `MemoryGroup`);
update crate AGENTS.md. Existing `Buffer::new`/`Image::new` unchanged.
3. **`renderer_rendergraph` (analysis)**: retain dependency DAG in
`CompiledGraph`; `happens_before` query; batch-mapped
`ResourceLifetime {first_batch, last_batch}`; replace `memory_offset: u64`
with a typed `Placement {offset, size, alignment, pool, aliased}`; imported
resources tracked but excluded from packing; queue-conservatism rule per D3.
4. **`renderer_rendergraph` (synchronization)**: `MemoryBarrier` +
`PipelineBarrier.memory_barriers`; recorder emits `vk::MemoryBarrier2` in
`DependencyInfo` (anchored via the existing `recording_steps`
`before_command` mechanism, like today's image/barrier steps); alias-barrier
construction per D4 (buffer and image variants, aggregation,
first-use-command anchoring); new `GraphError` variants for placement/bind
failures per D6, mapped from `renderer_resources::Error` at the
`resolve` boundary.
5. **`renderer_rendergraph` (placement)**: `plan_placements(device)` **replaces
`resolve()`'s current eager create+allocate loop** (graph.rs:1218-1276) for
pooled resources — create unbound, query requirements, group by a memory
type common to every member's `memoryTypeBits`, granularity-aware first-fit
over ranges (D4/D5, pool-per-class), allocate groups, bind, store
placements in `CompiledGraph`. Non-poolable resources (dedicated,
host-visible, imports) keep an allocate-and-bind-whole path equivalent to
today's. "Skip unused transients" lands here: created resources absent from
`lifetimes` are never created (no behavior change needed in
`record_commands`, which already skips ids absent from `lifetimes` at
graph.rs:1290). Transactional and idempotent; remove
`texture_size_estimate` from allocation decisions.
6. **Docs/tests**: update stale TODOs (`graph.rs:882`, `1315-1319`), document
the single-queue submission contract and the future multi-queue invariant
(alias transitions become semaphore edges).
## Verification
- Device-free unit tests: same-batch interference; unordered-but-distinct-batch
interference; proven-ordered reuse succeeds; granularity boundary (buffer
next to optimal image); range-overlap first-fit; dedicated/host-visible
exclusion; unused-transient skip; pooled-image non-UNDEFINED initial layout
rejected; alias-barrier scope correctness incl. read-only old occupant and
image `UNDEFINED` transition chaining.
- GPU-gated (`tests/graph_validation.rs`): buffer-buffer and image-image alias
reuse through the real barrier path, submit, `device_wait_idle`,
`assert_no_validation_errors`; teardown of shared allocation after all
resources drop.
- Commands: `cargo check --workspace`, `cargo test -p renderer_rendergraph`,
`cargo test -p renderer_resources --features testing`,
`cargo test -p renderer_device --features testing` (from workspace root,
pinned toolchain).
## Review history
- Drafted independently by deepseek/planner and luna/planner; cross-reviewed by
deepseek/reviewer and luna/reviewer (both verdicts: CHANGES REQUIRED).
- Blockers folded into this revision: batch-index ≠ happens-before (D1),
compile/resolve phase split (D2), `bufferImageGranularity` handling (D5),
alias-barrier/layout-transition chaining (D4), offset-aware bind APIs are new
work not existing capability (D6), queue model restricted to single-queue
(D3).
- Rev 3: diagnosis re-verified against code and `vk-mem 0.5.0` sources
(all six rows and the D6 API names confirmed). Follow-up comments folded in:
`happens_before` = compile-time transitive closure (D1), pool-per-class
granularity strategy picked (D5), pooled-image UNDEFINED initial-layout
rule (D5), pool groups keyed by intersecting `memoryTypeBits` (D6),
`MemoryGroup` transfers the `Arc` at bind time instead of retaining it (D6),
placement/bind error surfacing through `GraphError` (D6/task 4),
`plan_placements` explicitly replaces `resolve`'s eager loop (task 5).