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
203 lines
13 KiB
Markdown
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).
|