NanoVDB ReadAccessor::getDimAndActive - #2220
Conversation
Introduce a fused getDim + isActive traversal that shares the
Root->Internal->Leaf descent, the accessor cache update, and the
terminating-level mask read between the two queries. Adds a
DimAndActive POD, getDimAndActiveAndCache on Root/Internal/Leaf,
and getDimAndActive on all four ReadAccessor specializations.
Two policy tags control how `active` is resolved when an
InternalNode skip-flag fires along the descent:
- ActiveExact (default): preserves byte-for-byte semantics of
separate getDim + getAndCache<GetState>. `active` is correct
even when the skip-flag is set, at the cost of forcing the
child descent. Required by callers that read `active` at
upper levels (e.g. fvdb HDDA iterators).
- ActiveOnLeafOnly: cheap skip-flag short-circuit; `active` is
unspecified when the returned `dim` exceeds leaf-dim, and the
caller must gate on `dim` first
(`if (hdda.dim() > 1 || !active) continue;`).
HDDA.h ZeroCrossing is intentionally not migrated -- on the
register-tight sm_120 kernel, any fused shape grows reg/thread by
2-3 and drops occupancy from 4 to 3 blocks/SM. The kernel is
memory-latency bound (~14% achieved occupancy) and the L1 cache
absorbs the second descent at ~87% hit rate, so the occupancy hit
dominates the descent-fusion savings. Rationale recorded inline.
Also rework the ex_raytrace_level_set benchmark harness for honest
measurement: kernel-only cudaEvent timing on the CUDA path, 2
untimed warmup iters, min/median/mean/max reporting,
numIterations 50 -> 200, block size 512 -> 256.
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
The fused getDim+isActive traversal returns a {dim, active} pair. The
otherwise-natural {uint32_t, bool} struct layout passes through two
registers (or a packed 64-bit pair) at the function-return boundary.
NanoVDB's getDim return value is bounded by the largest internal-node
DIM in the default FloatTree hierarchy (4096 for an upper internal --
2^12, well below 2^31). The sign bit is unused, so we can pack:
low 31 bits = dim
sign bit = active
The struct keeps its existing API surface (aggregate-init via a 2-arg
constructor, accessors dim() and active()), so callers that used
`return {dim, active}` continue to work; consumers that previously
read `.dim` / `.active` as members switch to `.dim()` / `.active()`.
Effect on consumers:
- On the static ZeroCrossing kernel (which does not use the fused
API; rationale comment updated below): no change.
- On lean_hdda + fusion (a separate experiment) the renderOp kernel
drops from 62 regs (struct encoding) to 61 regs (packed encoding).
Doesn't cross an occupancy tier on sm_120 at b=256, so the wall-
time delta is within noise. Still useful: any consumer holding
DimAndActive live across other work gains 1 reg of headroom for
no semantic change.
- On fvdb's HDDA iterators (the productive consumer that reads
active unconditionally at upper levels), the packing reduces
return-boundary register pressure in a hot path.
Also expand the rationale comment in HDDA.h's ZeroCrossing. The
original justification framed the don't-migrate decision as a
register-cliff problem; testing against lean_hdda (which has 3 regs
of headroom for fusion) shows the same negative wall-time result
without an occupancy hit. The real reason is the second descent is
L1-warm at ~87 % hit rate, so eliminating it saves ~no cycles, and
the fused call's slightly heavier per-call overhead nets out slightly
worse. Verified both struct- and packed-encoded fusion across both
regimes.
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
The long NB block that explained why HDDA::ZeroCrossing keeps separate getDim + isActive calls (instead of using the fused API this branch adds) was useful during the iteration but doesn't earn its keep in the source tree -- a future reader doesn't need to relive the cliff/lean/descent-cost investigation that justified the decision. The rationale is preserved in the PR description: ZeroCrossing's `dim > 1 || !active` short-circuit makes the active read dead at upper levels, and the L1 cache absorbs the leaf-level descent at ~87% hit rate, so fusion saves essentially no cycles while adding a small per- call overhead. The fused API is retained for callers that read active unconditionally (fvdb's HDDA iterators -- -27% to -62% wall time win). Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
The DimAndActive constructor packed active via 'int32_t(a) << 31',
which shifts 1 into the sign bit of a signed int -- undefined behavior
under C++17 (the repo's minimum standard). Do the OR in uint32_t so the
shift is well-defined, and mask dim to 31 bits so a stray high bit can't
clobber the active bit.
Add TestNanoVDB.GetDimAndActive, which asserts the fused
getDimAndActive matches the separate getDim + isActive it replaces:
- ActiveExact: {dim, active} equals getDim(ijk, ray) and isActive(ijk)
byte-for-byte across a box spanning interior, surface and exterior.
- ActiveOnLeafOnly: dim always matches getDim; active matches isActive
whenever dim == 1 (the contract gate).
The sphere's narrow band is too thin to fill an InternalNode, so a
skip-flagged internal node above an active voxel cannot arise from the
geometry. The test forces that case by setting the skip-flag on the
lower node over a known-active voxel, then confirms ActiveExact descends
past it (active == true) while ActiveOnLeafOnly short-circuits
(active == false), both reporting the skipped dim.
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
There was a problem hiding this comment.
Pull request overview
This PR introduces a fused NanoVDB accessor query (ReadAccessor::getDimAndActive) that combines getDim(ijk, ray) and isActive(ijk) into a single root-to-leaf traversal to reduce per-iteration tree-walk cost in raymarch-style callers.
Changes:
- Add
DimAndActiveresult type plus policy tags (ActiveExact,ActiveOnLeafOnly) and implement fused traversal through Root/Internal/Leaf node descent and ReadAccessor entry points. - Add a unit test validating fused semantics against separate
getDim+isActive, including skip-flag behavior. - Update the raytrace level set example to run warmups and report min/median/mean/max timing statistics (and increase default iteration count).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
nanovdb/nanovdb/NanoVDB.h |
Adds DimAndActive + policy tags and implements getDimAndActive* traversal through the NanoVDB tree and accessors. |
nanovdb/nanovdb/unittest/TestNanoVDB.cc |
Adds unit test coverage for ReadAccessor::getDimAndActive under both policies, including skip-flag divergence cases. |
nanovdb/nanovdb/examples/ex_raytrace_level_set/nanovdb.cu |
Collects per-iteration timings with warmup iterations and prints min/median/mean/max stats. |
nanovdb/nanovdb/examples/ex_raytrace_level_set/common.h |
Adds CUDA-event-based kernel-only timing when compiled under NVCC and adjusts block size used by computeForEach. |
nanovdb/nanovdb/examples/ex_raytrace_level_set/main.cc |
Increases default number of iterations for the example run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- DimAndActive: store the packed field as uint32_t instead of int32_t. The previous int32_t form converted a uint32_t with bit 31 set to a signed int (implementation-defined) and left the defaulted ctor's member uninitialized. uint32_t packs/decodes with no conversion, the default ctor zero-initializes, and the size/ABI is unchanged. - ex_raytrace_level_set reportStats: take samples by const reference, early-return on an empty vector (front()/back() would otherwise be UB if numIterations were 0), and sort a local copy. - ex_raytrace_level_set common.h: check the cudaEventDestroy return codes with NANOVDB_CUDA_CHECK_ERROR, consistent with the rest of the CUDA timing path. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
The packed encoding masks dim to 31 bits (bit 31 holds active). dim is 1u << TOTAL, bounded by the deepest node dim (4096 for the default tree), so it always fits -- but a pathological tree config with TOTAL >= 31 would be silently truncated. Add a debug assert documenting the invariant, in the same style as the version-packing asserts above. Release builds are unaffected (assert compiles out; the mask still protects the active bit). Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
kmuseth
left a comment
kmuseth
left a comment
There was a problem hiding this comment.
Please address the inlined comments (related to very verbose documentation)
Other than that it looks good
Here's Calude's summary:
What it does: adds ReadAccessor::getDimAndActive(ijk, ray), fusing the HDDA step-size query (getDim) and the active-state query (isActive) into one root→leaf descent, returned as a bit-packed DimAndActive (dim in low 31 bits, active in bit 31 — keeps the return in one register). Two policy tags control behavior when an InternalNode's skip-flag is set: ActiveExact (byte-for-byte matches the separate getDim+isActive calls, at the cost of still descending past skip-flagged nodes to get the real active bit) and ActiveOnLeafOnly (short-circuits like plain getDim and leaves active contractually unspecified — for callers whose logic already discards active when dim > 1). Purely additive: no existing method's signature changes.
Verification performed:
Hand-traced getDimAndActiveAndCache at all three node levels (Root, Internal, Leaf) against the existing getDimAndCache + getAndCache it replaces, confirming algebraically that ActiveExact reproduces the exact original semantics (including the subtlety that the skip-flag short-circuits dim but must not short-circuit the active descent, since skip is set from value-range vs. render-delta, not from activeness) and that ActiveOnLeafOnly correctly mirrors getDim's cheap early-return when it takes that path.
Checked the DimAndActive packing for UB — none; the shift/mask are well-defined for uint32_t, and the constructor asserts dim < 2^31 (max real dim() is 4096).
Wrote a standalone reproduction of the PR's own GetDimAndActive unit test (not just read it) plus an additional dense cross-check test, compiled them against the PR's headers, and ran them on host: ~5.6M coordinate probes across both policies against directly-computed getDim/isActive, 0 mismatches, including the forced-skip-flag scenario where ActiveExact correctly reports the true active state while ActiveOnLeafOnly correctly reports the unspecified/false value — confirming the one place these two policies are supposed to diverge actually does.
Compiled the device-code path (global kernel calling getDimAndActive under both policies) to real SASS for sm_89 — clean, no errors.
Reviewed the ex_raytrace_level_set example/benchmark-infrastructure changes (warmup iterations, min/median/mean/max reporting, GPU event-based timing) — these back the PR's performance claims but don't call the new API themselves (consistent with the PR body: ZeroCrossing deliberately doesn't adopt it, since register-pressure/occupancy experiments showed no win there). Low risk either way since it's example code, not library surface.
Confirmed the pendingchanges entry accurately and concisely describes the change.
Findings: none. The implementation is a faithful, carefully-reasoned fusion of the existing traversal logic, the two-policy design honestly documents (and the code honestly implements) where the fast path trades away exactness, and my independent testing corroborates the correctness claims rather than just the PR's own numbers.
One minor observation, not a defect: at the LeafNode level, ActiveOnLeafOnly doesn't actually take a cheaper path when the leaf's own skip-flag is set — it always reads mValueMask.isOn(n) regardless of policy. This is explicitly documented as a deliberate tradeoff (a leaf-level branch on Policy was measured to cost 2 registers and cross an occupancy cliff, while leaf skip-flags are rare in practice) and stays within the "unspecified" contract (a correct value is a valid value for "unspecified"), so it's a documented engineering choice rather than a bug.
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Thanks @kmuseth, I addressed the long comments and reduced the total comment lines by ~60. |
kmuseth
left a comment
kmuseth
left a comment
There was a problem hiding this comment.
thanks for addressing my comments
…rs (#754) ## Summary OpenVDB PR [#2220](AcademySoftwareFoundation/openvdb#2220) added the fused `ReadAccessor::getDimAndActive` API to NanoVDB: a single Root→Internal→Leaf descent (or accessor-cache lookup) that returns both the HDDA step size and the active state of a voxel, packed into one register. This PR bumps the NanoVDB pin to that merge commit (`9df9ec06`) and adopts the fused call in both HDDA iterators, replacing the separate `getDim` + `isActive` pair that previously cost two tree walks per iteration. This restores the methodology originally prototyped alongside #663, which had to be reverted before merge because the fused accessor only existed in a fork at the time. ## Changes **`HDDASegmentIterator::nextSegment`** - One `getDimAndActive` call per iteration under the default `ActiveExact` policy, which is byte-for-byte equivalent to the separate calls. `ActiveExact` is required here because the iterator reads `active` unconditionally, including at coarse-tile levels. - When the HDDA re-aligns to a different level, `HDDA::update` snaps `mVoxel` to the new grid, so the result is re-queried after the update to keep `active` consistent with the voxel the TimeSpan logic reads. The two-descent cost is thus only paid on (rare) level-change iterations. **`HDDAValueIteratorImpl::nextVoxel`** - The realign-loop `getDim` queries and the post-convergence `isActive` fuse into one `getDimAndActive` per pass; each re-query runs at the voxel `HDDA::update` just snapped to, so on loop exit the result always describes the current voxel. The force-unrolled ≤3-pass structure is unchanged. - Policy is selected per instantiation: `HDDALeafVoxelIterator` (`LeafOnly=true`) gates on `dim == 1` before consulting `active`, which is exactly the `ActiveOnLeafOnly` contract, so it keeps `getDim`'s skip-flag fast path. `HDDAActiveValueIterator` (`LeafOnly=false`) must yield active coarse tiles, so it uses `ActiveExact`. No callers change — `ray_implicit_intersection`, `uniform_ray_samples`, `voxels_along_rays`, and `segments_along_rays` consume the iterators through their public API. ## Benchmarks RTX PRO 6000 Blackwell, 512×512 pinhole-camera rays, fp32, median of 20 CUDA-event-timed iterations (top/bottom-2 trimmed), NanoVDB example grids. Output checksums are bit-identical to main across all workloads — this is a pure performance change. | Op | Dataset | main (ms) | fused (ms) | Speedup | |---|---|---|---|---| | `voxels_along_rays` | dragon | 4.902 | 2.911 | **1.68×** | | `voxels_along_rays` | emu | 8.343 | 5.031 | **1.66×** | | `voxels_along_rays` | wdas_cloud | 24.140 | 16.970 | **1.42×** | | `voxels_along_rays` | crawler | 18.920 | 13.939 | **1.36×** | | `segments_along_rays` | crawler | 7.544 | 5.795 | **1.30×** | | `segments_along_rays` | emu | 3.703 | 2.900 | **1.28×** | | `segments_along_rays` | wdas_cloud | 12.676 | 9.945 | **1.27×** | | `segments_along_rays` | dragon | 1.634 | 1.351 | **1.21×** | | `uniform_ray_samples` | wdas_cloud | 12.798 | 10.033 | **1.28×** | | `uniform_ray_samples` | dragon | 1.669 | 1.385 | **1.21×** | | `ray_implicit_intersection` | emu | 0.587 | 0.478 | **1.23×** | | `ray_implicit_intersection` | crawler | 1.901 | 1.630 | **1.17×** | | `ray_implicit_intersection` | dragon | 0.384 | 0.374 | **1.03×** | `ray_implicit_intersection` gains the least because its leaf-only iterator already skips coarse tiles cheaply (and now additionally benefits from `ActiveOnLeafOnly` keeping the skip-flag fast path). ## Test plan - `pytest tests/unit/test_ray_marching.py tests/unit/test_sample.py` — 455 passed, 4 skipped. - `pytest tests/unit/test_basic_ops.py -k "ray or implicit or voxels_along or segments"` — 56 passed, 1 skipped, including the #692 zero-prefix regression test. - Benchmark outputs checksum-identical to main on all 13 workloads above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Jonathan Swartz <jonathan@jswartz.info> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
ReadAccessor::getDimAndActivefuses NanoVDB'sgetDimandisActiveinto a single root-to-leaf descent. For callers that read active unconditionally — fvdb-core'sHDDASegmentIteratorandHDDAVoxelIteratorare the applicable consumers — this collapses two tree walks per iteration into one.Measured on a Blackwell RTX 6000 Pro GPU across four NanoVDB example grids (dragon SDF, crawler SDF, emu SDF, WDAS cloud fog volume; 1024×1024 ray casts, median of 50 iters):
HDDASegmentIterator
HDDAVoxelIterator
Looking at ncu metrics, numbers drop in parallel to wall time improvements: fusion drops dynamic instructions by 26–47% and long-scoreboard stall by 8–9 percentage points on dragon. Register pressure drops 4–6 regs vs unfused (72 → 66–68) — no occupancy regression. The packed
DimAndActiveencoding (uint32 storage) keeps the return boundary at one register.Details on Trying to Use the Fused API in
HDDA::ZeroCrossingThe fused API is intentionally not used by
HDDA::ZeroCrossing. That kernel was benchmarked with the fused call across two register configurations on my Blackwell RTX 6000 Pro GPU — once on master (the existing kernel sits at exactly 64 regs/thread, on the occupancy cliff) and once on top of a separate refactor I did I'll call 'Lean HDDA' which was to free up register headroom (for additional registers needed by the dim/active fusion) by removing themT1, mDelta, mStepmembers from theHDDAclass and deriving them fromRaycomponents on each call in order to reduce register pressure (63 regs/thread, with 3 regs of headroom).On master (without 'lean HDDA'), active/dim fusion grew the kernel to 66–67 regs and dropped occupancy 50% → 37.5% (2 → 1 blocks/SM at 256 threads). On 'lean HDDA', the fusion fits under the cliff (61 regs with the packed encoding) without an occupancy hit (so we could measure fusion without an occupancy regression).
Wall time, however, is unchanged in both regimes — variants land within 0–1.3% of each other across sphere / dragon / crawler / emu, with the fused versions consistently a hair slower. The reason isn't the register/occupancy cliff: it's that
ZeroCrossing'sif (hdda.dim() > 1 || !acc.isActive(ijk)) continue;makes the active read dead at upper levels, and the L1 cache absorbs the second descent at ~87% hit rate on the leaf-level remainder. Eliminating it saves essentially no cycles, while the fused call's slightly heavier per-call overhead nets out marginally worse.The fused API is retained because it's a substantial win for callers that read active unconditionally — fvdb's HDDA iterators see −27% to −62% wall time and −26% to −47% dynamic instructions.
ZeroCrossing's pattern just isn't one of them.