NanoVDB: own PointsToGrid's device arrays with cuda::Buffer (CUDA) by harrism · Pull Request #2270 · AcademySoftwareFoundation/openvdb

harrism · 2026-08-05T04:03:11Z

Follow-up to #2231/#2251, part of #2232. This is B3, the last of step 2's scratch retrofit: PointsToGrid was the remaining builder with hand-paired allocate_async/deallocate_async calls — 47 of them.

Stacked on #2269 (which stacks on #2268) — review the last two commits: the loop prep and the conversion.

Commit 1 — express the density search as a loop

The bisection search over voxel size was a backward goto, which made the lifetimes of the buffers it retries over non-lexical. It becomes while (true) with continue on retry and break on convergence. Behavior-preserving; the loop body re-indents, so git diff -w shows the real change: 27 lines. Two locals (d_keys, d_node_count) carry results past the loop and hoist above it, as does the copy event — which was created inside the retried region on every iteration but destroyed once at the end, so each retry leaked an event handle; it is now created once.

Commit 2 — ownership

Design rule, refined from the note on the roadmap: owners own; raw pointers are views, refreshed immediately after every owner event (assignment, swap, destroy). The device-visible fields inside mData — which is memcpy'd to the device twice — are therefore never stale, replacing the previous unwritten invariant that nothing reads the device copy of d_indx in the ~185 lines between the two uploads. Released views are nulled, so a stale use faults instead of silently reading a freed block.

Members vs locals follow the pipeline. The inventory showed 9 of the 10 largest allocations are created in countNodes and freed in a different member function (processUpperNodes, processLeafNodes, processPoints, processBBox), so their owners are members; contained lifetimes are locals. All owners are Buffer<T, ResourceRef<ResourceT>>, borrowing the injected resource per the ownership rule of record (NanoVDB: injectable CUDA memory resources — design & roadmap #2232).
The index ping-pong becomes a swap of owners (mIndxBuf.swap(indxScratch)) instead of a bare pointer swap between a local and an uploaded struct field.
The retry path keeps its free-before-reallocate order via explicit destroy() calls, so peak device memory at retry is unchanged (move-assignment alone would briefly hold old + new).
Hand-matched byte sizes at every free site disappear; frees that sat one line before scope exit now just leave scope.
One behavior change, named: the too-many-points-per-leaf throw previously leaked its reduction scratch; ownership releases it during unwind.

Verification

Alloc/free parity gate (the roadmap's acceptance test for this conversion): a counting resource over three shapes — bulk segmented-sort branch (200k points, many tiles), serial per-tile branch (2k points, one tile), and the bisection retry engaged (PointsToGrid<Point>(4, 0, 8), 50k points):

case before (allocs/frees/bytes) after

bulk 18 / 18 / 16,499,187 identical

serial 19 / 19 / 117,720 identical

retry 38 / 38 / 6,579,539 identical
Full CUDA suite 52/53 (the failure is the pre-existing UnifiedBuffer_IO missing-data-file issue, identical on baseline); memory-resource suite 9/9; all instantiations (Point+custom, float+custom, Point+default) compile clean.
Per NanoVDB: please run the CUDA unit tests on a GPU in CI #2264, CI builds but does not run these — the numbers above are from a local RTX 6000 Ada, CUDA 12.6.

DistributedPointsToGrid (49 sites) stays deferred behind #2248. The remaining small builders (IndexToGrid 4, AddBlindData 2, GridStats 2, SignedFloodFill 2, VoxelBlockManager 1) and the sync-resource dispatch (step 2's vGPU acceptance criterion) are follow-ups.

Name the free operation destroy, as cuda::buffer does, and add the stream-taking overload it also provides. clear stays but only as a transitional delegate, marked as such: it exists because GridHandle::reset still calls it, and goes away with the legacy dual buffers that cuda::Buffer replaces. Rename setStream to set_stream. Member names cannot be aliased, so matching the standard spelling is the whole reason the resource concept kept allocate_async; the same argument applies here. Note in passing that ours deliberately does not synchronize, matching cuda::buffer's set_stream_unsynchronized rather than its set_stream, whose own documentation and implementation disagree (NVIDIA/cccl#10649). Add swap. The generic std::swap already does the right thing through the move operations, but both std::vector and rmm::device_buffer provide one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

Eleven of the builder's buffers are device-only -- nothing reads them on the host -- yet they were cuda::DeviceBuffer, whose host pointer and per-device array they never use. Move them to the single-space cuda::Buffer and give the builder a resource parameter, so its scratch is injectable like PointsToGrid's already is, and freed by scope rather than by hand. mProcessedRoot and mData are read on the host and stay dual. This is not a speedup: DeviceBuffer::init allocates host or device memory but never both, and these buffers always passed a real device id, so no cudaMallocHost was on this path to begin with. Measured dilate on 1k/20k/ 200k points, and the difference is within run-to-run noise. Hoist the Data struct out of the class. It does not depend on the resource, and leaving it nested would give every ResourceT its own incompatible type for the device functors to name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

A stream-ordered resource must also model the synchronous concept, which means writing four methods where two would do. The synchronous pair is not a bare delegate -- memory from allocate must be usable on any stream when it returns, so the null-stream allocation has to be synchronized first -- and omitting that yields memory which satisfies the concept but is not actually synchronous. Put it in one place rather than leaving each author to rediscover it. The two resources in TestMemoryResource are the first users, and were already wrong in exactly that way: they provide only the async pair, so they never modelled is_async_resource. TempPool duck-typed and never checked, so nothing caught it. MeshToGrid was the last builder allocating from a hard-wired DeviceResource, through TempDevicePool. Give it a ResourceT parameter and thread it into both its TopologyBuilder and its pool. As with Data in the builder, BoxTrianglePair is hoisted out of the class: it does not depend on the resource, and leaving it nested would give every ResourceT its own incompatible type for the device functors to name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

…uffer Buffer holds its resource by value, matching cuda::buffer -- whose model this completes: in CCCL the ownership semantics are selected by what is placed in the by-value slot, an owning any_resource or a borrowing resource_ref. We adopted the slot without the borrowing type, so a container like TempPool, whose contract is a non-owning pointer to a possibly stateful resource, had no way to hold a Buffer without copying that resource and stranding its state. ResourceRef is the missing piece: a non-owning reference that is itself a resource, so copying the ref shares the underlying instance. Its async methods exist only when R models AsyncResource, so a ref over a synchronous resource does not misreport its tier, and two refs compare equal exactly when they reference the same resource. TempPool now keeps its bytes in a Buffer<std::byte, ResourceRef<R>>: same resource contract, same stream retention, same discard-on-growth reallocation, but the block is freed by ownership rather than by hand. The TempPool unit tests, which assert traffic against the caller's own resource instance, pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

The bisection search over voxel size was written as a backward goto, which made the lifetimes of the buffers it retries over non-lexical. Rewrite it as while(true) with continue on retry and break on convergence; the six hand-written frees before the jump are unchanged. The change is easiest to review with whitespace ignored, since the loop body re-indents: git diff -w shows 27 changed lines. d_keys and d_node_count carry results past the loop, so their declarations move above it, as does the copy event, which was created inside the retried region on every iteration but destroyed only once at the end -- each retry leaked the previous handle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

Every device array PointsToGrid allocates is now owned by a Buffer<T, ResourceRef<ResourceT>> borrowing the injected resource -- members where the pipeline frees them in a later member function than the one that allocated them (countNodes allocates; processUpperNodes, processLeafNodes, processPoints and processBBox release), locals where the lifetime is contained. The raw pointers survive only as views: the device-visible fields inside mData, and the working pointers the cub and kernel calls take. Every owner event -- assignment, swap, destroy -- immediately refreshes its view, and released views are nulled so a stale use faults instead of reading a freed block. The index ping-pong becomes a swap of owners across the member/local boundary, replacing the bare pointer swap whose safety depended on nothing reading the device copy of d_indx between the two uploads. The density-search retry keeps its free-before-reallocate order via explicit destroy calls, so peak device memory is unchanged. The hand-matched byte sizes at every free site disappear, and the arrays released one line before scope exit now just leave scope. One behavior change worth naming: the too-many-points-per-leaf throw previously leaked the reduction scratch; ownership now releases it during unwind. Verified against the previous commit with a counting resource over three shapes (bulk segmented-sort branch, serial per-tile branch, and the bisection retry engaged): allocation count, free count, and total bytes are identical. Full CUDA and memory-resource suites unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>

Each release site sits in a function that receives the stream, so pass it to destroy rather than relying on the retained stream matching -- they are the same on every current path, but the explicit form does not depend on that staying true. Also drop the cudaGetDevice calls whose result the Buffer conversion left unused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com>