Skip to content

NanoVDB: merge and split single-space device grid handles, and deprecate the dual overloads (CUDA) - #2370

Merged
swahtz merged 9 commits into
AcademySoftwareFoundation:masterfrom
harrism:nanovdb-single-space-merge-split
Oct 20, 2026
Merged

swahtz merged 9 commits into
AcademySoftwareFoundation:masterfrom
harrism:nanovdb-single-space-merge-split

Conversation

@harrism

@harrism harrism commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Adds CUDA merge and split support for GridHandle<cuda::Buffer<std::byte, R>> and deprecates the dual-space overloads. This fills the gap reported by fvdb in #2232 and prepares for removing the dual-space buffers in 14.0, with a deprecation window starting in 13.2.

The new overloads take (handles, stream, proto), matching cuda::copyTo. They allocate the output through the prototype's resource and wait on each source buffer's retained stream. Grid indices, counts and head checksums are rewritten on the device, one thread per header and one kernel per 256 grids, followed by a single synchronization, so callers can release the sources on return. (Batched renumbering by @swahtz: tools::updateGridCount is now __hostdev__, so device and host renumbering share one function; for 300 grids this is 2 launches where per-grid kernels, readbacks and checksum passes took about 1,200.) The output reuses the source metadata at the new offsets without parsing the grids again on the device. MallocResource is supported on devices without memory pools.

The deprecated overloads name their replacements in the warning. Their implementations move to detail::, following #2301, so existing in-tree callers remain warning-free.

This also fixes two previously untested API problems:

  • The point-set overloads of tools::cuda::pointsToGrid and voxelsToGrid did not compile for either buffer type. They now build each set directly and merge with the appropriate implementation, without narrowing size_t to int.
  • The documented pointsToGrid(points, n, voxelSize, type, buffer, stream) overload had no definition. It is now implemented.

An audit of the public entry points in tools/cuda/ and cuda/ found no other missing definitions. A compile-and-link probe covers the previously uninstantiated entry points, using both buffer types where applicable.

Validation on one RTX PRO 6000 Blackwell GPU with nvcc 12.6:

  • New tests cover merge/split across streams, immediate source release, metadata, full checksums, byte-for-byte split results, balanced resource allocations, and completion on return for synchronous resources.
  • A 300-grid merge and split crosses the 256-grid batch boundary, with grids that have no checksum left empty.
  • Both buffer types are tested through the point-set APIs, along with the previously undefined pointsToGrid overload.
  • All single-GPU CUDA, buffer, memory-resource, and CPU ctest targets pass.
  • Examples, tools, and the Python module build with -Werror; all 38 Python tests pass.
  • An external caller receives deprecation warnings for both dual-space overloads.

harrism and others added 4 commits October 15, 2026 23:25
cuda::mergeGridHandles and cuda::splitGridHandles existed only for the
dual-space buffers, so a handle over cuda::Buffer<std::byte, R> -- the
storage the memory-resource API is built around -- had no merge or split,
and both functions would lose their only valid instantiation when the dual
buffers are removed.

Add single-space overloads taking (handles, stream, proto), the same
shape as cuda::copyTo. The output allocates through the prototype's
resource on the given stream, which is first ordered after each source
buffer's retained stream. One readback and one synchronization cover the
grid-count rewrite of all grids (the dual overloads synchronize per grid),
so the copies have completed on return and the caller may release the
sources immediately. The result adopts the source metadata at its new
offsets rather than re-parsing the grid chain on the device. Over a
synchronous resource the result is complete on return.

Also fix the point-set overloads of tools::cuda::pointsToGrid and
voxelsToGrid, which never compiled: they passed the stream where the
deprecated mergeDeviceGrids expected a buffer pointer, and the point-set
pointsToGrid called a voxel-size overload that was declared but never
defined. Define that overload and merge through cuda::mergeGridHandles for
either buffer family.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
The dual-space cuda::mergeGridHandles and splitGridHandles lose their only
valid instantiation when the dual buffers are removed, and now have a
single-space replacement, so deprecate them in the same release that adds
it.

Their bodies move to cuda::detail::mergeDualGridHandles and
splitDualGridHandles, and the public dual overloads become [[deprecated]]
forwarders, so the warning reaches only external code: the point-set
builder overloads at their default buffer type and the older
mergeDeviceGrids / splitDeviceGrids wrappers call the detail functions.
Those wrappers' messages now name the single-space replacement rather
than the deprecated overloads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
The single-space merge and split each built a Buffer<bool> over the
output's resource and passed rewriteGridCounts its raw data pointer,
with the required flag count stated only in a comment. Pass the output
buffer instead, so rewriteGridCounts allocates exactly one flag per
destination through the same resource and the two callers lose their
duplicated scratch setup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
… notes (CUDA)

The dual implementations moved into detail lost their hasDeviceDual
enable_if, and rewriteGridCounts became a template that needs a
single-space buffer: give all three a static_assert so a wrong buffer
type fails at the call with a message rather than deep in an
instantiation. Fold the dual implementations into the existing detail
block instead of closing and reopening it, and correct two notes that
said the point-set overloads merge through cuda::mergeGridHandles,
which is true only for the single-space family.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
@harrism
harrism requested a review from kmuseth as a code owner October 16, 2026 02:41
MSVC cannot deduce the template-template VectorT of
cuda::mergeGridHandles / detail::mergeDualGridHandles from a
std::vector argument (windows-nanovdb C2672), so pass the template
arguments explicitly, as the rest of the tree does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
@swahtz

swahtz commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Before this PR appeared, I'd prototyped single-space merge/split for openvdb/fvdb-core#770. It took a different approach to renumbering the headers. I'm not asking you to switch, but the trade-off might be useful here.

Renumber on the device with no readback. Today tools::updateGridCount is a host-only function, but its only callee, crc32Head(const GridData*), is already __hostdev__. If updateGridCount is also marked __hostdev__, one kernel thread can rewrite a header's index and count and recompute its head checksum in place. The grid pointers can travel by value in the kernel parameters, because 256 pointers take 2 KB and the guaranteed limit is 4 KB. So one launch covers 256 grids without any scratch allocation:

struct GridPtrBatch { static constexpr uint32_t Size = 256; GridData* ptr[Size]; };

static __global__ void updateGridCountBatch(GridPtrBatch batch, uint32_t n, uint32_t firstIndex,
                                            uint32_t indexStep, uint32_t gridCount)
{
    const uint32_t i = threadIdx.x;
    if (i < n) tools::updateGridCount(batch.ptr[i], firstIndex + i * indexStep, gridCount);
}

With indexStep = 1, a merge numbers its chain 0..N-1. With indexStep = 0, a split gives every output index 0.

For N grids the work compares as follows:

this PR device-side renumber
Renumber launches N, one thread each ⌈N/256⌉
Dirty-flag scratch 1 allocation none
Readback + cudaStreamSynchronize 1 0
Head-checksum refresh per changed grid: CRC LUT allocation, LUT kernel, crc32Head kernel done inside the renumber kernel
Merge copies 1 per grid 1 per source handle, since a handle's grids are contiguous

The cost of dropping the sync falls on lifetimes. Without it, the result is stream-ordered on stream, and callers can't release sources on return unless their frees are ordered after stream. That ordering is automatic for sources that retain stream, and it's the caller's job otherwise. A source over a synchronous resource has to stay alive until stream has consumed it. This PR's guarantee that sources can be released on return is simpler, so the change isn't a strict improvement. In exchange, with no sync, merge and split stay legal under stream capture when the resource is stream-ordered. That matters for batched callers like fvdb.

There's also a middle path. You could keep the sync, along with its release-on-return guarantee, and still do the renumbering and head checksums in the batched kernel. That removes the N launches, the per-grid LUT allocations, and the dirty-flag readback, and the contract stays as it is.

The event-based ordering after each source's retained stream in this PR is better than what my prototype did. My version left that ordering to the caller.

@swahtz

swahtz commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

For the paper trail: this addresses item 1 of the 2026-09-17 comment on #2232. #2371 covers item 2, the public adopt API.

Single-space mergeGridHandles and splitGridHandles rewrote each header's
grid index and count with one kernel per grid, read back per-grid dirty
flags, and refreshed each changed head checksum with its own CRC table
allocation and kernels.

tools::updateGridCount is now __hostdev__ (its only callee, crc32Head,
already was), so one thread per header renumbers it and recomputes its
head checksum in place. Header pointers travel by value in kernel
parameters, 256 per launch, so the renumber allocates no scratch and
needs no readback. The single synchronization stays, so sources may
still be released when the call returns.

Adds a 300-grid merge/split test that crosses the batch boundary and
mixes grids with and without checksums.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Comment thread nanovdb/nanovdb/cuda/GridHandle.cuh Outdated
Comment thread nanovdb/nanovdb/cuda/GridHandle.cuh Outdated
@swahtz

swahtz commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

A few optional nits, none blocking:

  • Event work in mergeGridHandles and splitGridHandles. The event is created and destroyed on every call, even when BufferT retains no stream and orderAfterRetainedStream does nothing. The merge also records and waits once per source handle, even when every source shares one stream. Guarding with if constexpr (BufferHasStream<BufferT>::value) and skipping streams already waited on would avoid both. This code likely changes anyway with the cross-device fix.
  • The grid-count overflow check runs after the stream-ordering loop. Moving it into the metadata loop, or at least above the ordering, means a rejected merge doesn't insert any cross-stream waits first.
  • The run-hint comment // make -j 4 testNanoVDB && ... now sits above voxelSetsToDeviceGrids. It belongs to CudaIndexGridToGrid_basic, so the new test should go above the comment.

cuda::mergeGridHandles and splitGridHandles created and destroyed an
event on every call, and the merge recorded and waited once per source
handle. RetainedStreamOrdering creates the event only when a wait is
needed and waits once per distinct retained stream. The merge now
rejects an oversized grid count before inserting any waits.

Also move the run-hint comment back above the test it belongs to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
@harrism

harrism commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @swahtz, all three taken in afa9724:

  • Event work: a small detail::RetainedStreamOrdering replaces the per-call event. It creates the event only when a wait is actually needed (never for a buffer type without a retained stream, or when every source already retains the target stream), and waits once per distinct retained stream, so a merge of handles sharing one stream inserts a single wait.
  • Grid-count check: now runs before the ordering, so a rejected merge inserts no waits.
  • Run-hint comment: back above CudaIndexGridToGrid_basic.

Full single-GPU suite passes.

harrism and others added 2 commits October 20, 2026 04:46
… ones (CUDA)

An event can only be recorded in its own device's context, so a source
whose retained stream belongs to another GPU made cudaEventRecord fail
and the process exit. Create and record each event on the retained
stream's device (queried from the stream where CUDA 12.8 allows it,
otherwise the device holding the storage), then wait on it from the
merge or split stream.

Empty sources (default, moved-from or reset handles) contribute no grids
and are no longer waited on, so they neither serialize the merge behind
the legacy default stream nor touch a stream that may have been
destroyed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

@swahtz swahtz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for addressing my notes

@swahtz
swahtz merged commit 8ec630f into AcademySoftwareFoundation:master Oct 20, 2026
16 checks passed
swahtz added a commit to swahtz/openvdb that referenced this pull request Oct 20, 2026
…::adopt

The single-space merge and split from AcademySoftwareFoundation#2370 read layouts through
GridHandle::metaData() and build their results with GridHandle::adopt
instead of the deprecated HandleFactory. mergeGridHandles rejects a grid
that would start at an unaligned offset before it allocates, since adopt
would otherwise reject the merged layout only after the copies.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
harrism added a commit to harrism/openvdb that referenced this pull request Oct 20, 2026
…eFoundation#2370) into nanovdb-resource-tool-api

Both sides defined pointsToGrid(points, count, voxelSize, type, buffer,
stream); keep this branch's definition, which adds the prototype-form
constraint and the argument checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Mark Harris <mharris@nvidia.com>
swahtz added a commit to swahtz/openvdb that referenced this pull request Oct 21, 2026
…::adopt

The single-space merge and split from AcademySoftwareFoundation#2370 read layouts through
GridHandle::metaData() and build their results with GridHandle::adopt
instead of the deprecated HandleFactory. mergeGridHandles rejects a grid
that would start at an unaligned offset before it allocates, since adopt
would otherwise reject the merged layout only after the copies.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
swahtz added a commit that referenced this pull request Oct 22, 2026
…2371)

* NanoVDB: add GridHandle::adopt and metaData, deprecate HandleFactory

GridHandle::adopt(buffer, meta) constructs a handle over bytes whose
layout the caller already knows: a copy of another handle's grids, or
grids the caller laid out itself. No grid header is read, so a device
buffer is wrapped without the parse kernel, readback and synchronization
the public constructor pays, and buffers over non-default-constructible
resources need no default construction. adopt checks only the layout
arithmetic: grids contiguous from offset zero, each starting at a
multiple of NANOVDB_DATA_ALIGNMENT, each at least a header, all within
the buffer, and an empty layout only over an empty buffer.
GridHandle::metaData() exposes the layout to pass back in. cuda::copyTo
and GridHandle::copy now use adopt.

cuda::detail::HandleFactory, which downstream code (fvdb) has been
reaching into for the same purpose, becomes a deprecated forwarder to
adopt and metaData().

mergeGrids and splitGrids also accept host-accessible single-space
handles such as GridHandle<cuda::Buffer<std::byte, PinnedResource>>,
allocating through the pool's resource or a default-constructed one.

New tests: GridHandleAdoptRejectsBadMeta covers each layout rejection
and a byte-for-byte adopt round trip; mergeSplitPinnedGrids checks
pinned merge and split against the HostBuffer result byte for byte.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

* NanoVDB: address review of GridHandle::adopt

mergeGrids and splitGrids walk grids by the handle's metadata and throw
when a grid header's size disagrees with it. Before, they sized from the
metadata but advanced by mGridSize, so an adopted layout smaller than its
headers overran the output buffer. splitGrids also builds its output with
push_back, so a buffer over a non-default-constructible resource can use
the pool path.

cuda::copyTo reuses the source's layout through detail::LayoutCopy
instead of adopt. The constructors accept grids at unaligned offsets
(e.g. Point grids written before #2329), and copyTo no longer rejects
them after the copy. The device GridHandle::copy() does the same. Public
adopt keeps its alignment check and now also rejects GridType::End.

Comments that overstated how much an adopted layout was validated are
corrected, and merge/split document that they do not synchronize
stream-ordered buffers.

Tests: mergeSplitRejectMismatchedMetaData (heap corruption before this
fix), CopyToKeepsUnalignedGrids, an End-type adopt case, and a
ResourceRef pinned merge/split with a pool.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

* NanoVDB: note HandleFactory::make's new layout checks in pending changes

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

* NanoVDB: switch cuda::mergeGridHandles/splitGridHandles to GridHandle::adopt

The single-space merge and split from #2370 read layouts through
GridHandle::metaData() and build their results with GridHandle::adopt
instead of the deprecated HandleFactory. mergeGridHandles rejects a grid
that would start at an unaligned offset before it allocates, since adopt
would otherwise reject the merged layout only after the copies.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

* NanoVDB: close CopyToKeepsUnalignedGrids after the merge with master

The merge resolution dropped the test's closing brace, so the next test
was parsed inside its body.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

* NanoVDB: build cuda::copyTo's empty prototype handle through GridHandle::adopt

The empty-source path that came in from master called the deprecated
HandleFactory, which fails builds with warnings as errors.

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>

---------

Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants