Skip to content

Commit 3f35101

Browse files
harrismclaude
andauthored
NanoVDB: run the builders on a synchronous memory resource (CUDA) (#2272)
* NanoVDB: align cuda::Buffer member names with cuda::buffer 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> * NanoVDB: allocate TopologyBuilder scratch from an injected resource 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> * NanoVDB: add SyncFromAsync, and give MeshToGrid a resource seam 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> * NanoVDB: add ResourceRef and route TempPool's scratch through cuda::Buffer 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> * NanoVDB: note cuda::Buffer and cuda::BufferView in pendingchanges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: express PointsToGrid's density search as a loop 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> * NanoVDB: own PointsToGrid's device arrays with cuda::Buffer 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> * NanoVDB: free TopologyBuilder scratch on the caller's stream 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> * NanoVDB: borrow TopologyBuilder scratch through ResourceRef The scratch buffers held their resource by value, so each of the eight carried its own copy -- fine for the stateless default, wrong for a stateful resource, whose accounting would be split across copies while the caller's instance saw nothing. Borrow through ResourceRef instead, the same reconciliation TempPool uses. Assert the stream-ordered requirement directly in TopologyBuilder and MeshToGrid so a synchronous-only resource fails with a diagnostic that names the builder, not just the pool inside it. Note SyncFromAsync's synchronize cost on its allocate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: guard PointsToGrid's copy event with a scope owner The too-many-points-per-leaf throw unwinds past the event's manual destroy, leaking the handle. Own it with a small guard so unwinding releases it, consistent with the buffer ownership in this function. Also assert the stream-ordered resource requirement on the class, so a synchronous-only resource fails naming PointsToGrid rather than the pool inside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: run the builders on a synchronous memory resource The builders require a stream-ordered resource, which a device without memory-pool support cannot provide through cudaMallocAsync. Two additions close the gap. MallocResource is a synchronous cudaMalloc/cudaFree resource that works on any device. AsyncFromSync presents a synchronous resource as a stream-ordered one -- the mirror of SyncFromAsync, and the analog of cuda::mr's synchronous_resource_adapter: allocate_async forwards, since synchronously allocated memory is already valid on every stream, and deallocate_async synchronizes the stream first, which makes the synchronous contract's quiescence requirement hold. The serialization cost of that synchronize is documented at the type, so a synchronous backend is an explicit caller choice rather than a silent substitution. The new test drives PointsToGrid end-to-end with an injected AsyncFromSync over a stateful synchronous resource: every scratch allocation routes through cudaMalloc and is freed through the caller's instance, never touching stream-ordered allocation -- which is exactly what a pool-less device requires of the scratch path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: assert the scratch alignment TopologyBuilder relies on The byte scratch is reinterpreted as word-sized types, which is valid for every resource whose DEFAULT_ALIGNMENT is at least word alignment -- all CUDA allocation paths give 256 -- but nothing said so. Assert it, so a custom resource with a weaker guarantee fails at compile time instead of misaligning on the device. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: terminate a bare cudaCheckError with a semicolon Legal without one -- the macro expands to a braced block -- but every other use in the file spells it as a statement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: make AsyncFromSync's null deallocation a no-op Null-free is a no-op everywhere else, so there is no quiescence to establish and no reason to synchronize the stream for it. Also correct the sync-resource test's comment: the grid handle's output buffer is allocated through BufferT, not the injected resource, so "every builder allocation" overstated what the test demonstrates -- it is the scratch allocations that never touch stream-ordered allocation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: set_stream matches cuda::buffer's contract, not a divergence cuda::buffer's set_stream is deliberately non-synchronizing -- its documented synchronization is a stale note left behind when the synchronization was removed -- so our non-synchronizing set_stream matches it in both name and contract, and the comment claiming a divergence was wrong. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: mark cuda::Buffer::clear deprecated in the documentation Documentation-level only: the [[deprecated]] attribute would warn from GridHandle::reset and NodeManager::reset, template members in our own headers that must keep calling clear() until every buffer type provides destroy() -- an unactionable diagnostic for callers of reset(), and a build break under -Werror. The attribute lands when those callers migrate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> * NanoVDB: exercise the concept-required members of the test resources The trait checks detect allocate/deallocate through unevaluated contexts, which never odr-use them, so nvcc warned that every synchronous pair and stub member was declared but never referenced (#177-D). The attribute route is closed -- nvcc's front end ignores [[maybe_unused]] for this diagnostic -- so reference them the honest way: a test that verifies the synchronous halves of the counting and stream-recording doubles behave like their stream-ordered halves, and one that pins the trait probes' stub behavior. The TU now builds with no warnings at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mark Harris <mharris@nvidia.com> --------- Signed-off-by: Mark Harris <mharris@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8a8ed7c commit 3f35101

3 files changed

Lines changed: 125 additions & 1 deletion

File tree

nanovdb/nanovdb/cuda/DeviceResource.h

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,79 @@ struct SyncFromAsync
192192
}
193193
};
194194

195+
/// @brief Synchronous device memory resource backed by cudaMalloc/cudaFree.
196+
/// Models only the Resource concept: it never touches stream-ordered
197+
/// allocation, so it works on devices without memory-pool support
198+
/// (cudaDevAttrMemoryPoolsSupported == 0), where DeviceResource's
199+
/// cudaMallocAsync path fails by design. Pair with AsyncFromSync to
200+
/// drive the stream-ordered builders on such a device.
201+
class MallocResource
202+
{
203+
public:
204+
// cudaMalloc aligns memory to 256 bytes by default
205+
static constexpr size_t DEFAULT_ALIGNMENT = 256;
206+
207+
/// @brief Allocates @c bytes with cudaMalloc; valid on every stream when
208+
/// this returns. A zero request returns nullptr.
209+
void* allocate(size_t bytes, size_t)
210+
{
211+
if (bytes == 0) return nullptr;
212+
void* p = nullptr;
213+
cudaCheck(cudaMalloc(&p, bytes));
214+
return p;
215+
}
216+
217+
/// @brief Frees @c p with cudaFree; the caller guarantees that device work
218+
/// touching the memory has completed.
219+
void deallocate(void* p, size_t, size_t) { cudaCheck(cudaFree(p)); }
220+
};// MallocResource
221+
222+
/// @brief Wrapper presenting a synchronous resource as a stream-ordered one,
223+
/// so it can drive components that require the AsyncResource concept
224+
/// (TempPool and the GPU builders).
225+
/// @tparam R the wrapped synchronous resource, held by value; wrap a
226+
/// ResourceRef<R> to borrow a stateful instance instead.
227+
/// @details The mirror of SyncFromAsync, and the analog of cuda::mr's
228+
/// synchronous_resource_adapter. allocate_async forwards to
229+
/// R::allocate, whose memory is immediately valid on every stream --
230+
/// a stronger guarantee than stream-ordering requires.
231+
/// deallocate_async synchronizes @c stream before R::deallocate,
232+
/// establishing the quiescence the synchronous contract demands.
233+
/// @warning Every deallocation synchronizes its stream, so expect
234+
/// serialization relative to a genuinely stream-ordered resource.
235+
/// That is the unavoidable cost of a synchronous backend under a
236+
/// stream-ordered algorithm; this wrapper exists so the cost is
237+
/// explicit and chosen by the caller -- e.g. on a device without
238+
/// memory-pool support -- rather than silently substituted.
239+
template<class R>
240+
struct AsyncFromSync
241+
{
242+
static_assert(is_resource<R>::value,
243+
"AsyncFromSync requires R to model the synchronous Resource concept");
244+
245+
static constexpr size_t DEFAULT_ALIGNMENT = R::DEFAULT_ALIGNMENT;
246+
247+
R resource;
248+
249+
/// @brief Allocates through the synchronous resource; the result is valid
250+
/// on every stream, hence trivially valid on @c stream.
251+
void* allocate_async(size_t bytes, size_t alignment, cudaStream_t) { return resource.allocate(bytes, alignment); }
252+
253+
/// @brief Synchronizes @c stream, then frees through the synchronous
254+
/// resource -- the synchronize makes the quiescence contract hold.
255+
/// Null is a no-op and skips the synchronize.
256+
void deallocate_async(void* p, size_t bytes, size_t alignment, cudaStream_t stream)
257+
{
258+
if (p == nullptr) return;
259+
cudaCheck(cudaStreamSynchronize(stream));
260+
resource.deallocate(p, bytes, alignment);
261+
}
262+
263+
/// @brief Synchronous pair, forwarding to the wrapped resource.
264+
void* allocate(size_t bytes, size_t alignment) { return resource.allocate(bytes, alignment); }
265+
void deallocate(void* p, size_t bytes, size_t alignment) { resource.deallocate(p, bytes, alignment); }
266+
};// AsyncFromSync<R>
267+
195268
/// @brief Non-owning reference to a memory resource that is itself a resource:
196269
/// copying the ref shares the underlying resource rather than copying it.
197270
/// @tparam R the referenced resource type

nanovdb/nanovdb/unittest/TestMemoryResource.cu

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,57 @@ TEST(TestMemoryResource, PinnedResource_DefaultResourceRoundTrip)
141141
// retained stream (the stream of the most recent reallocate), not the null stream.
142142
//======================================================================
143143

144+
// Synchronous-only and stateful: the shape of a vGPU or arena backend.
145+
struct SyncCountingResource
146+
{
147+
static constexpr size_t DEFAULT_ALIGNMENT = nanovdb::cuda::MallocResource::DEFAULT_ALIGNMENT;
148+
int allocs = 0, deallocs = 0;
149+
void* allocate(size_t bytes, size_t alignment) {
150+
void* p = nanovdb::cuda::MallocResource{}.allocate(bytes, alignment);
151+
if (p) ++allocs;
152+
return p;
153+
}
154+
void deallocate(void* p, size_t bytes, size_t alignment) {
155+
if (p) ++deallocs;
156+
nanovdb::cuda::MallocResource{}.deallocate(p, bytes, alignment);
157+
}
158+
};
159+
160+
static_assert(nanovdb::cuda::is_resource<nanovdb::cuda::MallocResource>::value,
161+
"MallocResource must model the synchronous Resource concept");
162+
static_assert(!nanovdb::cuda::is_async_resource<nanovdb::cuda::MallocResource>::value,
163+
"MallocResource must not claim the AsyncResource concept");
164+
static_assert(nanovdb::cuda::is_async_resource<
165+
nanovdb::cuda::AsyncFromSync<nanovdb::cuda::MallocResource>>::value,
166+
"AsyncFromSync must lift a synchronous resource to AsyncResource");
167+
168+
TEST(TestMemoryResource, PointsToGrid_RunsOnSynchronousResource)
169+
{
170+
// The pool-less-device path: every scratch allocation routes through
171+
// cudaMalloc/cudaFree via AsyncFromSync, never touching cudaMallocAsync.
172+
// The grid handle's output buffer is the exception -- getHandle allocates
173+
// it through BufferT::create, not through the injected resource.
174+
using RefT = nanovdb::cuda::ResourceRef<SyncCountingResource>;
175+
using VgpuT = nanovdb::cuda::AsyncFromSync<RefT>;
176+
SyncCountingResource base;
177+
VgpuT res{RefT(base)};
178+
179+
const std::vector<nanovdb::Coord> voxels = {{0,0,0},{1,2,3},{8,8,8},{100,100,100},{-50,20,7}};
180+
nanovdb::Coord* d_voxels = nullptr;
181+
ASSERT_EQ(cudaMalloc(&d_voxels, voxels.size()*sizeof(nanovdb::Coord)), cudaSuccess);
182+
ASSERT_EQ(cudaMemcpy(d_voxels, voxels.data(), voxels.size()*sizeof(nanovdb::Coord), cudaMemcpyHostToDevice), cudaSuccess);
183+
{
184+
nanovdb::tools::cuda::PointsToGrid<nanovdb::ValueOnIndex, VgpuT> converter(nanovdb::Map(1.0), cudaStream_t{0}, res);
185+
auto handle = converter.getHandle(d_voxels, voxels.size());
186+
auto* grid = handle.deviceGrid<nanovdb::ValueOnIndex>();
187+
EXPECT_NE(grid, nullptr);
188+
}
189+
ASSERT_EQ(cudaStreamSynchronize(0), cudaSuccess);
190+
ASSERT_EQ(cudaFree(d_voxels), cudaSuccess);
191+
EXPECT_GT(base.allocs, 0); // scratch really routed through the sync resource
192+
EXPECT_EQ(base.allocs, base.deallocs); // and every allocation was freed through it
193+
}
194+
144195
TEST(TestMemoryResource, TempPool_FreesOnRetainedStream)
145196
{
146197
cudaStream_t s = nullptr;

pendingchanges/nanovdb.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ NanoVDB:
55
- Added nanovdb::cuda::Buffer and nanovdb::cuda::BufferView (CUDA): a typed, resource-aware, stream-ordered container that allocates from an injectable memory resource and frees on its retained stream, and a non-owning view over externally managed memory that a GridHandle can wrap without copying. Member names follow cuda::buffer (destroy, set_stream, swap). Also added the synchronous resource concept nanovdb::cuda::is_resource alongside is_async_resource.
66

77
Improvements:
8-
- The GPU builders now allocate all scratch through an injectable memory resource: tools::cuda::TopologyBuilder and tools::cuda::MeshToGrid gained a ResourceT template parameter (defaulted, so existing code is unaffected), joining PointsToGrid. Added nanovdb::cuda::SyncFromAsync, a CRTP base that derives the synchronous half of the resource concept from the stream-ordered half, and nanovdb::cuda::ResourceRef, a non-owning reference to a resource that is itself a resource, for containers that hold their resource by value. PointsToGrid's device scratch and intermediate arrays are now owned by cuda::Buffer as well, replacing all of its hand-paired allocate/free calls.
8+
- The GPU builders now allocate all scratch through an injectable memory resource: tools::cuda::TopologyBuilder and tools::cuda::MeshToGrid gained a ResourceT template parameter (defaulted, so existing code is unaffected), joining PointsToGrid. Added nanovdb::cuda::SyncFromAsync, a CRTP base that derives the synchronous half of the resource concept from the stream-ordered half, and nanovdb::cuda::ResourceRef, a non-owning reference to a resource that is itself a resource, for containers that hold their resource by value. PointsToGrid's device scratch and intermediate arrays are now owned by cuda::Buffer as well, replacing all of its hand-paired allocate/free calls. Added nanovdb::cuda::MallocResource, a synchronous cudaMalloc-backed resource that works on devices without memory-pool support, and nanovdb::cuda::AsyncFromSync, which presents any synchronous resource as a stream-ordered one by synchronizing before each deallocation, so the builders can run with an injected synchronous resource.
99
- Added the CMake option NANOVDB_CUDA_WERROR (default ON, also implied by OPENVDB_CXX_STRICT) which passes --Werror=all-warnings to NVCC so that every device-side diagnostic becomes an error when building the NanoVDB tests, tools and examples. Disable it if a newer CUDA toolkit introduces diagnostics that block your build.
1010
- The bug-fix to the nanovdb::ReadAccessor (see below) improves random-access performance in some use-cases (especially on the CPU).
1111

0 commit comments

Comments
 (0)