From e39e75f87c88dcececcba1fb596be4a1468d3ca6 Mon Sep 17 00:00:00 2001 From: afender Date: Wed, 3 Feb 2021 13:27:08 -0600 Subject: [PATCH 1/8] enabled linear accessor features --- include/rmm/cuda_stream_pool.hpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index 803c0474e..573ff4685 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -61,6 +61,33 @@ class cuda_stream_pool { return streams_[(next_stream++) % streams_.size()].view(); } + /** + * @brief Get a `cuda_stream_view` of the stream at stream_id index the pool. + * + * This function is thread safe with respect to other calls to the same function. + * + * @throws rmm::out_of_range exception if `stream_index >= size()` + * + * @param stream_id The index of the stream in the pool + * + * @return rmm::cuda_stream_view + */ + rmm::cuda_stream_view get_stream(std::size_t stream_index) const + { + RMM_EXPECTS( + stream_id < streams_.size(), rmm::out_of_range, "Attempt to access out of bounds stream."); + return streams_[stream_index].view(); + } + + /** + * @brief Get the number of streams in the pool. + * + * This function is thread safe with respect to other calls to the same function. + * + * @return the number of streams in the pool + */ + size_t get_pool_size() const noexcept { return streams_.size(); } + private: std::vector streams_; mutable std::atomic_size_t next_stream{}; From 9d8f571933a6f86efea7a04c80264f5b8f8bc808 Mon Sep 17 00:00:00 2001 From: afender Date: Wed, 3 Feb 2021 14:09:52 -0600 Subject: [PATCH 2/8] added tests --- include/rmm/cuda_stream_pool.hpp | 3 ++- tests/cuda_stream_pool_tests.cpp | 21 ++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index 573ff4685..15b5e8a04 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -75,7 +76,7 @@ class cuda_stream_pool { rmm::cuda_stream_view get_stream(std::size_t stream_index) const { RMM_EXPECTS( - stream_id < streams_.size(), rmm::out_of_range, "Attempt to access out of bounds stream."); + stream_index < streams_.size(), rmm::out_of_range, "Attempt to access out of bounds stream."); return streams_[stream_index].view(); } diff --git a/tests/cuda_stream_pool_tests.cpp b/tests/cuda_stream_pool_tests.cpp index 9ca2a4188..30d21c971 100644 --- a/tests/cuda_stream_pool_tests.cpp +++ b/tests/cuda_stream_pool_tests.cpp @@ -54,7 +54,26 @@ TEST_F(CudaStreamPoolTest, ValidStreams) RMM_CUDA_TRY(cudaMemsetAsync(v.data(), 0xcc, 100, stream_a.value())); stream_a.synchronize(); - auto v2 = rmm::device_uvector{v, stream_b}; + auto v2 = rmm::device_uvector{v, stream_b}; auto x = v2.front_element(stream_b); EXPECT_EQ(x, 0xcc); } + +TEST_F(CudaStreamPoolTest, PoolSize) { EXPECT_GE(this->pool.get_pool_size(), 1); } + +TEST_F(CudaStreamPoolTest, OutOfBoundLinearAccess) +{ + EXPECT_NO_THROW(this->pool.get_stream(this->pool.get_pool_size() - 1)); + EXPECT_THROW(this->pool.get_stream(this->pool.get_pool_size()), rmm::out_of_range); +} + +TEST_F(CudaStreamPoolTest, ValidLinearAccess) +{ + auto const stream_a = this->pool.get_stream(0); + auto const stream_b = this->pool.get_stream(1); + EXPECT_NE(stream_a, stream_b); + EXPECT_FALSE(stream_a.is_default()); + EXPECT_FALSE(stream_a.is_per_thread_default()); + EXPECT_FALSE(stream_b.is_default()); + EXPECT_FALSE(stream_b.is_per_thread_default()); +} \ No newline at end of file From 2977343f2ed9c09e9d45a7c64021fb447f0f0178 Mon Sep 17 00:00:00 2001 From: Alex Fender Date: Fri, 5 Feb 2021 15:15:47 -0600 Subject: [PATCH 3/8] Update cuda_stream_pool_tests.cpp --- tests/cuda_stream_pool_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cuda_stream_pool_tests.cpp b/tests/cuda_stream_pool_tests.cpp index 30d21c971..6b62625cd 100644 --- a/tests/cuda_stream_pool_tests.cpp +++ b/tests/cuda_stream_pool_tests.cpp @@ -76,4 +76,4 @@ TEST_F(CudaStreamPoolTest, ValidLinearAccess) EXPECT_FALSE(stream_a.is_per_thread_default()); EXPECT_FALSE(stream_b.is_default()); EXPECT_FALSE(stream_b.is_per_thread_default()); -} \ No newline at end of file +} From 898cf5d0acefbb5dcf026d9bf9c76fb830f8fdc5 Mon Sep 17 00:00:00 2001 From: afender Date: Mon, 8 Feb 2021 12:26:27 -0600 Subject: [PATCH 4/8] addressed review comments --- include/rmm/cuda_stream_pool.hpp | 9 +++------ tests/cuda_stream_pool_tests.cpp | 5 +++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index 15b5e8a04..4bfde5533 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -64,20 +64,17 @@ class cuda_stream_pool { /** * @brief Get a `cuda_stream_view` of the stream at stream_id index the pool. + * Equivalent values of `stream_id` return a stream_view to the same underlying stream. * * This function is thread safe with respect to other calls to the same function. * - * @throws rmm::out_of_range exception if `stream_index >= size()` - * * @param stream_id The index of the stream in the pool * * @return rmm::cuda_stream_view */ - rmm::cuda_stream_view get_stream(std::size_t stream_index) const + rmm::cuda_stream_view get_stream(std::size_t stream_id) const { - RMM_EXPECTS( - stream_index < streams_.size(), rmm::out_of_range, "Attempt to access out of bounds stream."); - return streams_[stream_index].view(); + return streams_[stream_id % streams_.size()].view(); } /** diff --git a/tests/cuda_stream_pool_tests.cpp b/tests/cuda_stream_pool_tests.cpp index 30d21c971..80b518590 100644 --- a/tests/cuda_stream_pool_tests.cpp +++ b/tests/cuda_stream_pool_tests.cpp @@ -63,8 +63,9 @@ TEST_F(CudaStreamPoolTest, PoolSize) { EXPECT_GE(this->pool.get_pool_size(), 1); TEST_F(CudaStreamPoolTest, OutOfBoundLinearAccess) { - EXPECT_NO_THROW(this->pool.get_stream(this->pool.get_pool_size() - 1)); - EXPECT_THROW(this->pool.get_stream(this->pool.get_pool_size()), rmm::out_of_range); + auto const stream_a = this->pool.get_stream(0); + auto const stream_b = this->pool.get_stream(this->pool.get_pool_size()); + EXPECT_EQ(stream_a, stream_b); } TEST_F(CudaStreamPoolTest, ValidLinearAccess) From bff998b2fced047de07f409cbf4fd91ae6cbfa1d Mon Sep 17 00:00:00 2001 From: Alex Fender Date: Mon, 8 Feb 2021 12:42:25 -0600 Subject: [PATCH 5/8] Update include/rmm/cuda_stream_pool.hpp Co-authored-by: Jake Hemstad --- include/rmm/cuda_stream_pool.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index 4bfde5533..035fc779f 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -63,7 +63,7 @@ class cuda_stream_pool { } /** - * @brief Get a `cuda_stream_view` of the stream at stream_id index the pool. + * @brief Get a `cuda_stream_view` of the stream associated with `stream_id`. * Equivalent values of `stream_id` return a stream_view to the same underlying stream. * * This function is thread safe with respect to other calls to the same function. From d084d04d586e0e43f11cb45eb323373e72123731 Mon Sep 17 00:00:00 2001 From: Alex Fender Date: Mon, 8 Feb 2021 12:42:34 -0600 Subject: [PATCH 6/8] Update include/rmm/cuda_stream_pool.hpp Co-authored-by: Jake Hemstad --- include/rmm/cuda_stream_pool.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index 035fc779f..cf0b85655 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -68,7 +68,7 @@ class cuda_stream_pool { * * This function is thread safe with respect to other calls to the same function. * - * @param stream_id The index of the stream in the pool + * @param stream_id Unique identifier for the desired stream * * @return rmm::cuda_stream_view */ From 9c3e1bc6aef241d287730b5da26cc6877d3b31bc Mon Sep 17 00:00:00 2001 From: afender Date: Mon, 8 Feb 2021 15:01:18 -0600 Subject: [PATCH 7/8] simplified test --- tests/cuda_stream_pool_tests.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/cuda_stream_pool_tests.cpp b/tests/cuda_stream_pool_tests.cpp index 6807177a1..1e14e2abf 100644 --- a/tests/cuda_stream_pool_tests.cpp +++ b/tests/cuda_stream_pool_tests.cpp @@ -73,8 +73,4 @@ TEST_F(CudaStreamPoolTest, ValidLinearAccess) auto const stream_a = this->pool.get_stream(0); auto const stream_b = this->pool.get_stream(1); EXPECT_NE(stream_a, stream_b); - EXPECT_FALSE(stream_a.is_default()); - EXPECT_FALSE(stream_a.is_per_thread_default()); - EXPECT_FALSE(stream_b.is_default()); - EXPECT_FALSE(stream_b.is_per_thread_default()); } From fa8634350ef8f081e8c2e0785e3913d164a3c1e2 Mon Sep 17 00:00:00 2001 From: afender Date: Mon, 8 Feb 2021 16:03:43 -0600 Subject: [PATCH 8/8] clang style --- include/rmm/cuda_stream_pool.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/rmm/cuda_stream_pool.hpp b/include/rmm/cuda_stream_pool.hpp index cf0b85655..2e77f2047 100644 --- a/include/rmm/cuda_stream_pool.hpp +++ b/include/rmm/cuda_stream_pool.hpp @@ -63,12 +63,12 @@ class cuda_stream_pool { } /** - * @brief Get a `cuda_stream_view` of the stream associated with `stream_id`. + * @brief Get a `cuda_stream_view` of the stream associated with `stream_id`. * Equivalent values of `stream_id` return a stream_view to the same underlying stream. * * This function is thread safe with respect to other calls to the same function. * - * @param stream_id Unique identifier for the desired stream + * @param stream_id Unique identifier for the desired stream * * @return rmm::cuda_stream_view */