Skip to content

Commit c230ecf

Browse files
authored
[CUDAX] Remove synchronization from set_stream and add a stream argument to destroy in async_buffer (#5697)
* Remove synchronization from set_stream and add a stream argument to destroy * Fix format * Adjust comments
1 parent 1bf9fcd commit c230ecf

File tree

3 files changed

+29
-15
lines changed

3 files changed

+29
-15
lines changed

cudax/include/cuda/experimental/__container/async_buffer.cuh

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ public:
550550
}
551551

552552
//! @brief Returns the stored stream
553+
//! @note Stream used to allocate the buffer is initially stored in the buffer, but can be changed with `set_stream`
553554
[[nodiscard]] _CCCL_HIDE_FROM_ABI constexpr stream_ref stream() const noexcept
554555
{
555556
return __buf_.stream();
@@ -559,15 +560,6 @@ public:
559560
//! @param __new_stream the new stream
560561
//! @note Always synchronizes with the old stream
561562
_CCCL_HIDE_FROM_ABI constexpr void set_stream(stream_ref __new_stream)
562-
{
563-
__buf_.set_stream(__new_stream);
564-
}
565-
566-
//! @brief Replaces the stored stream
567-
//! @param __new_stream the new stream
568-
//! @warning This does not synchronize between \p __new_stream and the current stream. It is the user's responsibility
569-
//! to ensure proper stream order going forward
570-
_CCCL_HIDE_FROM_ABI constexpr void set_stream_unsynchronized(stream_ref __new_stream) noexcept
571563
{
572564
__buf_.set_stream_unsynchronized(__new_stream);
573565
}
@@ -596,6 +588,15 @@ public:
596588
}
597589

598590
//! @brief Destroys the async_buffer, deallocates the buffer and destroys the memory resource
591+
//! @param __stream The stream to deallocate the buffer on.
592+
//! @warning After this explicit destroy call, the buffer can only be assigned to or destroyed.
593+
_CCCL_HIDE_FROM_ABI void destroy(::cuda::stream_ref __stream)
594+
{
595+
__buf_.destroy(__stream);
596+
}
597+
598+
//! @brief Destroys the async_buffer, deallocates the buffer and destroys the memory resource
599+
//! @note Uses the stored stream to deallocate the buffer, equivalent to calling buffer.destroy(buffer.stream())
599600
//! @warning After this explicit destroy call, the buffer can only be assigned to or destroyed.
600601
_CCCL_HIDE_FROM_ABI void destroy()
601602
{

cudax/include/cuda/experimental/__container/uninitialized_async_buffer.cuh

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,20 +249,32 @@ public:
249249
}
250250

251251
//! @brief Destroys an \c uninitialized_async_buffer, deallocates the buffer in stream order on the stream that
252-
//! was used to create the buffer and destroys the memory resource.
252+
//! is stored in the buffer and destroys the memory resource.
253+
//! @param __stream The stream to deallocate the buffer on.
253254
//! @warning destroy does not destroy any objects that may or may not reside within the buffer. It is the
254255
//! user's responsibility to ensure that all objects within the buffer have been properly destroyed.
255-
_CCCL_HIDE_FROM_ABI void destroy()
256+
_CCCL_HIDE_FROM_ABI void destroy(::cuda::stream_ref __stream)
256257
{
257258
if (__buf_)
258259
{
259-
__mr_.deallocate(__stream_, __buf_, __get_allocation_size(__count_));
260+
__mr_.deallocate(__stream, __buf_, __get_allocation_size(__count_));
260261
__buf_ = nullptr;
261262
__count_ = 0;
262263
}
264+
// TODO should we make sure we move the mr only once by moving it to the if above?
265+
// It won't work for 0 count buffers, so we would probably need a separate bool to track it
263266
auto __tmp_mr = ::cuda::std::move(__mr_);
264267
}
265268

269+
//! @brief Destroys an \c uninitialized_async_buffer, deallocates the buffer in stream order on the stream that
270+
//! is stored in the buffer and destroys the memory resource.
271+
//! @warning destroy does not destroy any objects that may or may not reside within the buffer. It is the
272+
//! user's responsibility to ensure that all objects within the buffer have been properly destroyed.
273+
_CCCL_HIDE_FROM_ABI void destroy()
274+
{
275+
destroy(__stream_);
276+
}
277+
266278
//! @brief Destroys an \c uninitialized_async_buffer and deallocates the buffer in stream order on the stream
267279
//! that was used to create the buffer.
268280
//! @warning The destructor does not destroy any objects that may or may not reside within the buffer. It is the
@@ -331,6 +343,7 @@ public:
331343
}
332344

333345
//! @brief Returns the stored stream
346+
//! @note Stream used to allocate the buffer is initially stored in the buffer, but can be changed with `set_stream`
334347
[[nodiscard]] _CCCL_HIDE_FROM_ABI constexpr ::cuda::stream_ref stream() const noexcept
335348
{
336349
return __stream_;

cudax/test/containers/async_buffer/access.cu

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,12 @@ C2H_CCCLRT_TEST("cudax::async_buffer access and stream", "[container][async_buff
9595

9696
{
9797
cudax::stream other_stream{cuda::device_ref{0}};
98-
buf.set_stream_unsynchronized(other_stream);
98+
buf.set_stream(other_stream);
9999
CUDAX_CHECK(buf.stream() == other_stream);
100-
// TODO swap to synchronized setter one cudax::stream_ref is moved to cuda namespace
101-
buf.set_stream_unsynchronized(stream);
100+
buf.set_stream(stream);
102101
}
103102

104103
CUDAX_CHECK(buf.stream() == stream);
104+
buf.destroy(stream);
105105
}
106106
}

0 commit comments

Comments
 (0)