Repository navigation
NanoVDB: merge and split single-space device grid handles, and deprecate the dual overloads (CUDA) - #2370
Conversation
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>
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>
|
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 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 For N grids the work compares as follows:
The cost of dropping the sync falls on lifetimes. Without it, the result is stream-ordered on 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. |
|
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>
|
A few optional nits, none blocking:
|
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>
|
Thanks @swahtz, all three taken in afa9724:
Full single-GPU suite passes. |
… 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
left a comment
There was a problem hiding this comment.
Looks good, thanks for addressing my notes
…::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>
…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>
…::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>
…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>
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), matchingcuda::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::updateGridCountis 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.MallocResourceis 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:
tools::cuda::pointsToGridandvoxelsToGriddid not compile for either buffer type. They now build each set directly and merge with the appropriate implementation, without narrowingsize_ttoint.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/andcuda/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:
pointsToGridoverload.-Werror; all 38 Python tests pass.