perf(rt): high-water-mark growth for TLAS host-input buffers (#64) #98

Merged
catbot merged 1 commit from claude/issue-64 into master 2026-06-16 20:24:49 +02:00
Member

Summary

RenderingElement3D::BuildTLAS reallocated the host-visible instanceBuffer and metadataBuffer on every topology change (instance-count or build-flag change). Those buffers only ever need to hold at least primitiveCount entries — the AS build reads exactly primitiveCount of them via tlasRangeInfo, and the copy loop writes [0, primitiveCount) — so a shrink, or any growth that still fits the previous capacity, can reuse the existing allocation.

This gates the two Resize calls behind a high-water-mark check, removing two of the four reallocations on a count change. The AS storage + scratch rebuild (the dominant cost on this path) is unchanged, since it is tied to the AS itself.

Correctness

  • The build reads primitiveCount instances regardless of buffer capacity, so an oversized instance buffer is spec-correct — verified with the validation layer (zero errors).
  • No new lifetime hazards: the change only reduces reallocation frequency. As noted in the issue, the frame is drained by wait-idle today, so there is no UAF; a frame-in-flight renderer would still need fence-tracked lifetimes (unchanged by this PR).
  • Mirrors the WebGPU backend, which already allocates these mirrors once and never resizes (for an analogous driver-stability reason).

Tests

Adds tests/TLASHighWaterMark, driving the real hardware AS-build path (cube BLAS + BuildTLAS at a sequence of instance counts). It asserts:

  • first build allocates; growth past capacity reallocates (new handle/address/size);
  • shrink reuses the existing allocation unchanged (the core win — the pre-fix code reallocated here);
  • within-capacity growth and exactly-at-capacity growth reuse it (boundary: > not >=);
  • a same-count rebuild takes the refit path with builtInstanceCount unchanged;
  • builtInstanceCount tracks the live count across every build;
  • zero Vulkan validation errors throughout.

crafter-build test — all 14 tests pass (run with --jobs=1; the parallel runner has a pre-existing git pull race on the shared external-dep cache, unrelated to this change).

Resolves #64

## Summary `RenderingElement3D::BuildTLAS` reallocated the host-visible `instanceBuffer` and `metadataBuffer` on every topology change (instance-count or build-flag change). Those buffers only ever need to hold **at least** `primitiveCount` entries — the AS build reads exactly `primitiveCount` of them via `tlasRangeInfo`, and the copy loop writes `[0, primitiveCount)` — so a shrink, or any growth that still fits the previous capacity, can reuse the existing allocation. This gates the two `Resize` calls behind a high-water-mark check, removing **two of the four** reallocations on a count change. The AS storage + scratch rebuild (the dominant cost on this path) is unchanged, since it is tied to the AS itself. ## Correctness - The build reads `primitiveCount` instances regardless of buffer capacity, so an oversized instance buffer is spec-correct — verified with the validation layer (zero errors). - No new lifetime hazards: the change only *reduces* reallocation frequency. As noted in the issue, the frame is drained by wait-idle today, so there is no UAF; a frame-in-flight renderer would still need fence-tracked lifetimes (unchanged by this PR). - Mirrors the WebGPU backend, which already allocates these mirrors once and never resizes (for an analogous driver-stability reason). ## Tests Adds `tests/TLASHighWaterMark`, driving the real hardware AS-build path (cube BLAS + `BuildTLAS` at a sequence of instance counts). It asserts: - first build allocates; growth past capacity reallocates (new handle/address/size); - **shrink reuses** the existing allocation unchanged (the core win — the pre-fix code reallocated here); - within-capacity growth and exactly-at-capacity growth reuse it (boundary: `>` not `>=`); - a same-count rebuild takes the refit path with `builtInstanceCount` unchanged; - `builtInstanceCount` tracks the live count across every build; - zero Vulkan validation errors throughout. `crafter-build test` — all 14 tests pass (run with `--jobs=1`; the parallel runner has a pre-existing `git pull` race on the shared external-dep cache, unrelated to this change). Resolves #64
BuildTLAS reallocated the host-visible instanceBuffer and metadataBuffer
on every topology change. They only ever need to hold at least
primitiveCount entries (the AS build reads exactly primitiveCount via
tlasRangeInfo, and the copy loop writes [0, primitiveCount)), so a
shrink — or any growth that still fits the previous capacity — can reuse
the existing allocation. Gate the two Resize calls on a high-water-mark
check, removing two of the four reallocations on a count change. The AS
storage + scratch rebuild below is unchanged: it is tied to the AS
itself and dominates this path regardless.

Adds tests/TLASHighWaterMark driving the real hardware AS-build path: it
asserts a shrink (and within-capacity growth) reuses the buffers while a
growth past the high-water reallocates, that builtInstanceCount still
tracks the live count, and that feeding an oversized instance buffer to
the build produces zero Vulkan validation errors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
catbot merged commit 00b84dd301 into master 2026-06-16 20:24:49 +02:00
catbot deleted branch claude/issue-64 2026-06-16 20:24:49 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Catcrafts/Crafter.Graphics!98
No description provided.