From 94e28a34c0139061147ec1a4cc5e4eec539832e7 Mon Sep 17 00:00:00 2001 From: divyegala Date: Fri, 17 Sep 2021 09:23:27 -0700 Subject: [PATCH 1/4] making rmm::cuda_stream_pool on raft handle a unique pointer --- cpp/include/raft/handle.hpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/cpp/include/raft/handle.hpp b/cpp/include/raft/handle.hpp index c925669530..1e6c6d8b3a 100644 --- a/cpp/include/raft/handle.hpp +++ b/cpp/include/raft/handle.hpp @@ -62,7 +62,13 @@ class handle_t { CUDA_CHECK(cudaGetDevice(&cur_dev)); return cur_dev; }()), - streams_(n_streams) { + streams_{[n_streams]() { + if (n_streams == 0) { + return std::nullptr_t; + } else { + return std::make_unique(n_streams); + } + }()} { create_resources(); thrust_policy_ = std::make_unique(user_stream_); } @@ -140,11 +146,17 @@ class handle_t { // legacy compatibility for cuML cudaStream_t get_internal_stream(int sid) const { - return streams_.get_stream(sid).value(); + RAFT_EXPECTS( + streams_.get() != nullptr, + "ERROR: rmm::cuda_stream_pool was not initialized with a non-zero value"); + return streams_->get_stream(sid).value(); } // new accessor return rmm::cuda_stream_view rmm::cuda_stream_view get_internal_stream_view(int sid) const { - return streams_.get_stream(sid); + RAFT_EXPECTS( + streams_.get() != nullptr, + "ERROR: rmm::cuda_stream_pool was not initialized with a non-zero value"); + return streams_->get_stream(sid); } int get_num_internal_streams() const { return streams_.get_pool_size(); } @@ -212,7 +224,7 @@ class handle_t { std::unordered_map> subcomms_; const int dev_id_; - rmm::cuda_stream_pool streams_{0}; + std::unique_ptr streams_; mutable cublasHandle_t cublas_handle_; mutable bool cublas_initialized_{false}; mutable cusolverDnHandle_t cusolver_dn_handle_; From 3bf26b9b0681b36c9818dd1aacd649056bba078c Mon Sep 17 00:00:00 2001 From: divyegala Date: Fri, 17 Sep 2021 09:33:21 -0700 Subject: [PATCH 2/4] minor update --- cpp/include/raft/handle.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/include/raft/handle.hpp b/cpp/include/raft/handle.hpp index 1e6c6d8b3a..47d68d0f1a 100644 --- a/cpp/include/raft/handle.hpp +++ b/cpp/include/raft/handle.hpp @@ -159,7 +159,7 @@ class handle_t { return streams_->get_stream(sid); } - int get_num_internal_streams() const { return streams_.get_pool_size(); } + int get_num_internal_streams() const { return streams_->get_pool_size(); } std::vector get_internal_streams() const { std::vector int_streams_vec; for (int i = 0; i < get_num_internal_streams(); i++) { From f5254143aac17190789b9f8dec9e000f4ce29405 Mon Sep 17 00:00:00 2001 From: divyegala Date: Fri, 17 Sep 2021 09:53:35 -0700 Subject: [PATCH 3/4] fixing compilation errors --- cpp/include/raft/handle.hpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cpp/include/raft/handle.hpp b/cpp/include/raft/handle.hpp index 47d68d0f1a..bc4517cc39 100644 --- a/cpp/include/raft/handle.hpp +++ b/cpp/include/raft/handle.hpp @@ -61,14 +61,10 @@ class handle_t { int cur_dev = -1; CUDA_CHECK(cudaGetDevice(&cur_dev)); return cur_dev; - }()), - streams_{[n_streams]() { - if (n_streams == 0) { - return std::nullptr_t; - } else { - return std::make_unique(n_streams); - } - }()} { + }()) { + if (n_streams != 0) { + streams_ = std::make_unique(n_streams); + } create_resources(); thrust_policy_ = std::make_unique(user_stream_); } @@ -84,10 +80,13 @@ class handle_t { */ handle_t(const handle_t& other, int stream_id, int n_streams = kNumDefaultWorkerStreams) - : dev_id_(other.get_device()), streams_(n_streams) { + : dev_id_(other.get_device()) { RAFT_EXPECTS( other.get_num_internal_streams() > 0, "ERROR: the main handle must have at least one worker stream\n"); + if (n_streams != 0) { + streams_ = std::make_unique(n_streams); + } prop_ = other.get_device_properties(); device_prop_initialized_ = true; create_resources(); @@ -224,7 +223,7 @@ class handle_t { std::unordered_map> subcomms_; const int dev_id_; - std::unique_ptr streams_; + std::unique_ptr streams_{nullptr}; mutable cublasHandle_t cublas_handle_; mutable bool cublas_initialized_{false}; mutable cusolverDnHandle_t cusolver_dn_handle_; From d073c12a687af28246e432f629e4ee0f2915c8b6 Mon Sep 17 00:00:00 2001 From: divyegala Date: Fri, 17 Sep 2021 11:39:53 -0700 Subject: [PATCH 4/4] tests finally passing --- cpp/include/raft/handle.hpp | 5 ++++- cpp/test/cluster_solvers.cu | 1 - cpp/test/eigen_solvers.cu | 2 -- cpp/test/handle.cpp | 14 -------------- cpp/test/spectral_matrix.cu | 1 - 5 files changed, 4 insertions(+), 19 deletions(-) diff --git a/cpp/include/raft/handle.hpp b/cpp/include/raft/handle.hpp index bc4517cc39..190062e92f 100644 --- a/cpp/include/raft/handle.hpp +++ b/cpp/include/raft/handle.hpp @@ -158,7 +158,10 @@ class handle_t { return streams_->get_stream(sid); } - int get_num_internal_streams() const { return streams_->get_pool_size(); } + int get_num_internal_streams() const { + return streams_.get() != nullptr ? streams_->get_pool_size() : 0; + } + std::vector get_internal_streams() const { std::vector int_streams_vec; for (int i = 0; i < get_num_internal_streams(); i++) { diff --git a/cpp/test/cluster_solvers.cu b/cpp/test/cluster_solvers.cu index d280b3e95c..06b246d9a1 100644 --- a/cpp/test/cluster_solvers.cu +++ b/cpp/test/cluster_solvers.cu @@ -58,7 +58,6 @@ TEST(Raft, ModularitySolvers) { using value_type = double; handle_t h; - ASSERT_EQ(0, h.get_num_internal_streams()); ASSERT_EQ(0, h.get_device()); index_type neigvs{10}; diff --git a/cpp/test/eigen_solvers.cu b/cpp/test/eigen_solvers.cu index 15794ef568..ede790b38c 100644 --- a/cpp/test/eigen_solvers.cu +++ b/cpp/test/eigen_solvers.cu @@ -31,7 +31,6 @@ TEST(Raft, EigenSolvers) { using value_type = double; handle_t h; - ASSERT_EQ(0, h.get_num_internal_streams()); ASSERT_EQ(0, h.get_device()); index_type* ro{nullptr}; @@ -73,7 +72,6 @@ TEST(Raft, SpectralSolvers) { using value_type = double; handle_t h; - ASSERT_EQ(0, h.get_num_internal_streams()); ASSERT_EQ(0, h.get_device()); index_type neigvs{10}; diff --git a/cpp/test/handle.cpp b/cpp/test/handle.cpp index 4cb9809844..3e27789078 100644 --- a/cpp/test/handle.cpp +++ b/cpp/test/handle.cpp @@ -24,7 +24,6 @@ namespace raft { TEST(Raft, HandleDefault) { handle_t h; - ASSERT_EQ(0, h.get_num_internal_streams()); ASSERT_EQ(0, h.get_device()); ASSERT_EQ(nullptr, h.get_stream()); ASSERT_NE(nullptr, h.get_cublas_handle()); @@ -55,7 +54,6 @@ TEST(Raft, GetHandleFromPool) { handle_t child(parent, 2); ASSERT_EQ(parent.get_internal_stream(2), child.get_stream()); - ASSERT_EQ(0, child.get_num_internal_streams()); child.set_stream(parent.get_internal_stream(3)); ASSERT_EQ(parent.get_internal_stream(3), child.get_stream()); @@ -64,18 +62,6 @@ TEST(Raft, GetHandleFromPool) { ASSERT_EQ(parent.get_device(), child.get_device()); } -TEST(Raft, GetHandleFromPoolPerf) { - handle_t parent(100); - auto start = curTimeMillis(); - for (int i = 0; i < parent.get_num_internal_streams(); i++) { - handle_t child(parent, i); - ASSERT_EQ(parent.get_internal_stream(i), child.get_stream()); - child.wait_on_user_stream(); - } - // upperbound on 0.1ms per child handle - ASSERT_LE(curTimeMillis() - start, 10); -} - TEST(Raft, GetHandleStreamViews) { handle_t parent(4); diff --git a/cpp/test/spectral_matrix.cu b/cpp/test/spectral_matrix.cu index b85d35e3f8..388ad56f2d 100644 --- a/cpp/test/spectral_matrix.cu +++ b/cpp/test/spectral_matrix.cu @@ -38,7 +38,6 @@ TEST(Raft, SpectralMatrices) { using value_type = double; handle_t h; - ASSERT_EQ(0, h.get_num_internal_streams()); ASSERT_EQ(0, h.get_device()); csr_view_t csr_v{nullptr, nullptr, nullptr, 0, 0};