DRAFT/diagnostic: accessor benchmark never reaches a leaf node (all lookups resolve tiles) - #12
Draft
sifakis wants to merge 1 commit into
Conversation
This is a diagnostic change, not a merge candidate. It adds an assert
immediately before every getValue() in both CUDA kernels, probing the
same coordinate via grid->tree().root().probeLeaf() -- deliberately
going through the root rather than the accessor, so accessor cache
state cannot influence the result.
Both benchmarks run to completion with zero assertion failures:
bench_accessor_cuda_new : exit=0, 0 assertion failures
bench_accessor_cuda_old : exit=0, 0 assertion failures
That is, no lookup in any of the five workloads (Sequential, LeafJump,
NodeJump, Random, 27-point Stencil) touches a leaf node, under either
accessor type and either accessor mode.
Why: createFogVolumeSphere is constant through its interior, so the
interior is stored as active *tiles*, not leaves -- 82,608 leaves x 512
= 42.3M voxels against 523.6M active, i.e. ~92% of active voxels are
tiles. The leaf shell lives only at radius ~[480,520), while DOMAIN=256
confines every sample to the tile interior. Measured termination levels
are 100% upper-internal (L2) for Sequential and 99.8% L2 / 0.2% lower
(L1) for Random; never level 0.
Consequence: the reported figures compare a cached upper-node pointer
comparison against a lookup in an 8-entry root table, both L1-resident,
one hop below the root. They do not measure voxel access or leaf-cache
reuse. This also explains why ReadAccessor<0> is nearly flat across
Sequential/LeafJump/NodeJump regardless of stride -- its leaf cache can
never hit, because no leaf exists in the sampled region.
Note RESULTS.md:76-79 states the opposite ("every lookup lands on a real
leaf and traverses the tree for real").
Verification: each assert was checked live by inverting its predicate
and confirming it fires (the Release build defines NDEBUG, which is why
the local #undef is required -- without it both asserts compile away
silently and the run passes vacuously).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Efty Sifakis <esifakis@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — diagnostic only, not intended for merge. The
#undef NDEBUGdeliberatelyre-enables asserts in Release builds, which you would never want merged. This exists so the
check can be reproduced rather than taken on trust.
What this shows
The benchmark never reads a voxel. Every lookup terminates on an active tile in an
internal node — no lookup in any of the five workloads reaches a leaf.
The change adds one assert immediately before every
getValue()in both CUDA kernels,probing the same coordinate via
grid->tree().root().probeLeaf(). The probe deliberatelygoes through the root rather than the accessor, so accessor cache state cannot influence
the result.
Clean under both accessor types (
<0,1,2>and<0>) and both accessor modes (OLD and NEW),on an RTX 5090 / nvcc 13.0, covering Sequential, LeafJump, NodeJump, Random and the 27-point
Stencil.
Why
createFogVolumeSphereis constant through its interior, so the interior is stored as activetiles rather than leaves:
The leaf shell exists only at radius ~[480, 520), while
DOMAIN = 256(BenchPatterns.h:26)confines every sample to the tile interior — the cube's farthest corner is at radius
256·√3 ≈ 443. Measured termination levels: Sequential 100% upper-internal (L2);
Random 99.8% L2 / 0.2% lower (L1). Never level 0.
Consequence for the results
The reported figures compare a cached upper-node pointer comparison against a lookup in an
8-entry root table — both L1-resident, one hop below the root. They do not measure voxel
access or leaf-cache reuse, so they do not bear on the AcademySoftwareFoundation#2260 fix in either direction. (The
correctness fix itself is unaffected; this is only about what the benchmark measures.)
This also explains the table's most conspicuous oddity:
ReadAccessor<0>is nearly flat at2.23 / 2.48 / 3.23 ns across Sequential / LeafJump / NodeJump regardless of stride. Its leaf
cache can never hit, because no leaf exists in the sampled region, so every access is the
same root descent no matter the pattern.
Note
RESULTS.md:76-79currently states the opposite: "every lookup lands on a real leaf andtraverses the tree for real — we are not measuring empty-space shortcuts." The "100% active"
half is correct; the "lands on a real leaf" half is 0%. The comment at
BenchPatterns.h:23-26shows the hazard was anticipated —
DOMAINwas chosen to avoid escaping into empty space —but avoiding the background tile landed the sampling in the interior tile.
Verification that the asserts are live
A clean pass is meaningless if the assert were compiled out or unreachable, so each was
checked by inverting its predicate and confirming it fires:
benchKernelinverted → fires immediately on every threadstencilKernelinverted → 122 failures at line 67, exit 1, while the point patterns stillpass (isolating it to the stencil kernel, so the two asserts are verified independently)
The Release build defines
NDEBUG, which is why the local#undefis required — without itboth asserts compile away silently and the run passes vacuously.
Reproducing
cmake -S . -B build -DOPENVDB_BUILD_CORE=OFF -DOPENVDB_BUILD_NANOVDB=ON \ -DNANOVDB_BUILD_BENCHMARK=ON -DNANOVDB_USE_CUDA=ON -DCMAKE_BUILD_TYPE=Release cmake --build build --target bench_accessor_cuda_new bench_accessor_cuda_old -j ./build/nanovdb/nanovdb/benchmark/bench_accessor_cuda_newThe same result was independently confirmed on the CPU by instrumenting
BenchAccessor.cc:631,242,752
getValue()calls across both accessor types and both builds, 100.0000% withprobeLeaf() == nullptr.Separate, unrelated note
benchmark/CMakeLists.txt:17,29defineNANOVDB_NO_OLD_ACCESSOR, which does not existanywhere in the tree — it appears to be left over from an earlier revision of AcademySoftwareFoundation#2260 before the
default was flipped. It is currently harmless (the
_newtargets get NEW behaviour becauseNANOVDB_USE_OLD_ACCESSORis simply undefined), but configuring with-DNANOVDB_USE_OLD_ACCESSOR=ONwould propagate that macro through thenanovdbINTERFACEtarget to both bench targets, silently making
bench_accessor_newan OLD build. Notincluded here to keep this PR purely diagnostic — happy to send it separately.