Skip to content

Commit 586dde7

Browse files
harrismclaude
andauthored
NanoVDB: inject a resource into TopologyBuilder scratch, and align cuda::Buffer with cuda::buffer (CUDA) (#2268)
* 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: 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: 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: 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 bb597a2 commit 586dde7

7 files changed

Lines changed: 287 additions & 134 deletions

File tree

nanovdb/nanovdb/cuda/Buffer.h

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ struct StreamHolder<true> { cudaStream_t mStream = 0; };
5050
/// is_resource). When @c R provides both interfaces the stream-ordered
5151
/// one is used.
5252
/// @details With a stream-ordered resource the Buffer retains the stream of
53-
/// the most recent allocation (or the one supplied via setStream)
53+
/// the most recent allocation (or the one supplied via set_stream)
5454
/// and orders its deallocation on that stream. Buffer is move-only.
5555
template<typename T, typename R = DeviceResource>
5656
class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
@@ -133,7 +133,7 @@ class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
133133
Buffer& operator=(Buffer&& other) noexcept
134134
{
135135
if (this != &other) {
136-
this->clear();
136+
this->destroy();
137137
static_cast<detail::StreamHolder<IsAsync>&>(*this) = other;
138138
mResource = std::move(other.mResource);
139139
mData = other.mData;
@@ -168,7 +168,7 @@ class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
168168

169169
/// @brief D-tor. A stream-ordered resource frees on the retained stream;
170170
/// a synchronous resource frees immediately.
171-
~Buffer() { this->clear(); }
171+
~Buffer() { this->destroy(); }
172172

173173
/// @brief Returns the retained stream, i.e. the stream the buffer's memory
174174
/// will be freed on.
@@ -179,9 +179,11 @@ class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
179179
/// deallocation (and destruction) is ordered on @c stream instead.
180180
/// @param stream cuda stream subsequent deallocation is ordered on
181181
/// @warning The caller is responsible for ordering @c stream after any
182-
/// in-flight work that uses the buffer's memory.
182+
/// in-flight work that uses the buffer's memory. This deliberately
183+
/// does not synchronize, matching cuda::buffer's set_stream, which
184+
/// avoids implicit synchronization in fundamental primitives.
183185
template<typename S = R, std::enable_if_t<is_async_resource<S>::value, int> = 0>
184-
void setStream(cudaStream_t stream) { this->mStream = stream; }
186+
void set_stream(cudaStream_t stream) { this->mStream = stream; }
185187

186188
/// @brief Resizes the buffer to @c count elements, preserving the leading
187189
/// min(old, new) elements. Every operation — the new allocation, the
@@ -219,7 +221,7 @@ class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
219221
mSize = count;
220222
}
221223
else {
222-
this->mStream = stream; // no reallocation: setStream semantics
224+
this->mStream = stream; // no reallocation: set_stream semantics
223225
}
224226
}
225227

@@ -263,13 +265,50 @@ class Buffer : private detail::StreamHolder<is_async_resource<R>::value>
263265
bool empty() const { return mSize == 0; }
264266

265267
/// @brief Frees the buffer memory (if any) and resets to the empty state.
266-
void clear()
268+
/// A stream-ordered resource frees on the retained stream.
269+
/// @note Spelled destroy to match cuda::buffer. This is the name to use.
270+
void destroy()
267271
{
268272
this->deallocate(mData, mSize);
269273
mData = nullptr;
270274
mSize = 0;
271275
}
272276

277+
/// @brief Frees the buffer memory (if any) and resets to the empty state.
278+
/// @deprecated Use destroy(). Documentation-level only for now: the
279+
/// [[deprecated]] attribute would fire from GridHandle::reset
280+
/// and NodeManager::reset, which must keep calling clear()
281+
/// until every buffer type provides destroy().
282+
/// @note Transitional, and not the name to use: it exists only because
283+
/// GridHandle::reset still calls clear() on its buffer. It goes away
284+
/// when the legacy dual buffers do and GridHandle moves to destroy().
285+
void clear() { this->destroy(); }
286+
287+
/// @brief Frees the buffer memory (if any) on @c stream and resets to the
288+
/// empty state. @c stream becomes the retained stream.
289+
/// @param stream cuda stream the deallocation is ordered on
290+
/// @warning The caller is responsible for ordering @c stream after any
291+
/// in-flight work that uses the buffer's memory.
292+
template<typename S = R, std::enable_if_t<is_async_resource<S>::value, int> = 0>
293+
void destroy(cudaStream_t stream)
294+
{
295+
this->mStream = stream;
296+
this->destroy();
297+
}
298+
299+
/// @brief Exchanges the contents of this buffer with @c other. Neither
300+
/// buffer allocates, frees, or copies element data.
301+
/// @param other buffer to exchange contents with
302+
void swap(Buffer& other) noexcept
303+
{
304+
auto& lhs = static_cast<detail::StreamHolder<IsAsync>&>(*this);
305+
auto& rhs = static_cast<detail::StreamHolder<IsAsync>&>(other);
306+
std::swap(lhs, rhs);
307+
std::swap(mResource, other.mResource);
308+
std::swap(mData, other.mData);
309+
std::swap(mSize, other.mSize);
310+
}
311+
273312
private:
274313
/// @brief Returns @c count * sizeof(T), throwing std::runtime_error if the
275314
/// byte size would overflow size_t instead of silently wrapping into

nanovdb/nanovdb/tools/cuda/CoarsenGrid.cuh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ void CoarsenGrid<BuildT>::coarsenInternalNodes()
210210
if (auto srcLeafCount = mSrcTreeData.mNodeCount[0]) { // Unless it's an empty grid
211211
util::cuda::lambdaKernel<<<numBlocks(srcLeafCount), mNumThreads, 0, mStream>>>(
212212
srcLeafCount, util::morphology::cuda::CoarsenInternalNodesFunctor<BuildT>(),
213-
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mBuilder.mUpperMasks.deviceData(), mBuilder.mLowerMasks.deviceData() );
213+
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mBuilder.mUpperMasks.data(), mBuilder.mLowerMasks.data() );
214214
}
215215
}// CoarsenGrid<BuildT>::coarsenInternalNodes
216216

nanovdb/nanovdb/tools/cuda/PruneGrid.cuh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ void PruneGrid<BuildT>::pruneInternalNodes()
213213
if (auto srcLeafCount = mSrcTreeData.mNodeCount[0]) { // Unless it's an empty grid
214214
util::cuda::lambdaKernel<<<numBlocks(srcLeafCount), mNumThreads, 0, mStream>>>(
215215
srcLeafCount, util::morphology::cuda::PruneInternalNodesFunctor<BuildT>(),
216-
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mDeviceSrcLeafMask, mBuilder.mUpperMasks.deviceData(), mBuilder.mLowerMasks.deviceData() );
216+
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mDeviceSrcLeafMask, mBuilder.mUpperMasks.data(), mBuilder.mLowerMasks.data() );
217217
}
218218
}// PruneGrid<BuildT>::pruneInternalNodes
219219

nanovdb/nanovdb/tools/cuda/RefineGrid.cuh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ void RefineGrid<BuildT>::refineInternalNodes()
225225
if (auto srcLeafCount = mSrcTreeData.mNodeCount[0]) { // Unless it's an empty grid
226226
util::cuda::lambdaKernel<<<numBlocks(srcLeafCount), mNumThreads, 0, mStream>>>(
227227
srcLeafCount, util::morphology::cuda::RefineInternalNodesFunctor<BuildT>(),
228-
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mBuilder.mUpperMasks.deviceData(), mBuilder.mLowerMasks.deviceData() );
228+
mDeviceSrcGrid, mBuilder.deviceProcessedRoot(), mBuilder.mUpperMasks.data(), mBuilder.mLowerMasks.data() );
229229
}
230230
}// RefineGrid<BuildT>::refineInternalNodes
231231

0 commit comments

Comments
 (0)