From 2e9cb10043e48f770eb6719babd755af732015fa Mon Sep 17 00:00:00 2001 From: Nghia Truong Date: Mon, 29 Jul 2024 10:51:43 -0700 Subject: [PATCH 1/5] Make `get` a `const` function and `set` thread-safe Signed-off-by: Nghia Truong --- cpp/include/cudf/utilities/prefetch.hpp | 8 +++++++- cpp/src/utilities/prefetch.cpp | 13 ++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cpp/include/cudf/utilities/prefetch.hpp b/cpp/include/cudf/utilities/prefetch.hpp index 49fca73a2c8..d794ef7d015 100644 --- a/cpp/include/cudf/utilities/prefetch.hpp +++ b/cpp/include/cudf/utilities/prefetch.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -47,13 +48,17 @@ class prefetch_config { /** * @brief Get the value of a configuration key. * + * If the key does not exist, a `false` value will be returned. + * * @param key The configuration key. * @return The value of the configuration key. */ - bool get(std::string_view key); + bool get(std::string_view key) const; /** * @brief Set the value of a configuration key. * + * This is a thread-safe operation. + * * @param key The configuration key. * @param value The value to set. */ @@ -68,6 +73,7 @@ class prefetch_config { private: prefetch_config() = default; //< Private constructor to enforce singleton pattern std::map config_values; //< Map of configuration keys to values + std::mutex config_mtx; //< Mutex for thread-safe config access }; /** diff --git a/cpp/src/utilities/prefetch.cpp b/cpp/src/utilities/prefetch.cpp index 16f2c3a1202..fe8c195e046 100644 --- a/cpp/src/utilities/prefetch.cpp +++ b/cpp/src/utilities/prefetch.cpp @@ -32,15 +32,18 @@ prefetch_config& prefetch_config::instance() return instance; } -bool prefetch_config::get(std::string_view key) +bool prefetch_config::get(std::string_view key) const { // Default to not prefetching - if (config_values.find(key.data()) == config_values.end()) { - return (config_values[key.data()] = false); - } + if (config_values.find(key.data()) == config_values.end()) { return false; } return config_values[key.data()]; } -void prefetch_config::set(std::string_view key, bool value) { config_values[key.data()] = value; } + +void prefetch_config::set(std::string_view key, bool value) +{ + std::scoped_lock lock(config_mtx); + config_values[key.data()] = value; +} cudaError_t prefetch_noexcept(std::string_view key, void const* ptr, From 2d18e7adf784ea01faa6479c641f06c6f815c953 Mon Sep 17 00:00:00 2001 From: Nghia Truong Date: Mon, 29 Jul 2024 10:55:10 -0700 Subject: [PATCH 2/5] Use `map::get` Signed-off-by: Nghia Truong --- cpp/src/utilities/prefetch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/utilities/prefetch.cpp b/cpp/src/utilities/prefetch.cpp index fe8c195e046..23417c88032 100644 --- a/cpp/src/utilities/prefetch.cpp +++ b/cpp/src/utilities/prefetch.cpp @@ -36,7 +36,7 @@ bool prefetch_config::get(std::string_view key) const { // Default to not prefetching if (config_values.find(key.data()) == config_values.end()) { return false; } - return config_values[key.data()]; + return config_values.get(key.data()); } void prefetch_config::set(std::string_view key, bool value) From 79372f3696258fd18f7ed9dc2601dc4b58308161 Mon Sep 17 00:00:00 2001 From: Nghia Truong Date: Mon, 29 Jul 2024 11:13:58 -0700 Subject: [PATCH 3/5] Fix map access and make thread-safe get Signed-off-by: Nghia Truong --- cpp/src/utilities/prefetch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/src/utilities/prefetch.cpp b/cpp/src/utilities/prefetch.cpp index 23417c88032..bf803e788a1 100644 --- a/cpp/src/utilities/prefetch.cpp +++ b/cpp/src/utilities/prefetch.cpp @@ -34,9 +34,10 @@ prefetch_config& prefetch_config::instance() bool prefetch_config::get(std::string_view key) const { + std::scoped_lock lock(config_mtx); // Default to not prefetching if (config_values.find(key.data()) == config_values.end()) { return false; } - return config_values.get(key.data()); + return config_values.at(key.data()); } void prefetch_config::set(std::string_view key, bool value) From 8ba0763ca602e3d17fc798c80f5331d9b8bd9f69 Mon Sep 17 00:00:00 2001 From: Nghia Truong Date: Mon, 29 Jul 2024 11:23:24 -0700 Subject: [PATCH 4/5] Use `shared_mutex` and `shared_lock` Signed-off-by: Nghia Truong --- cpp/include/cudf/utilities/prefetch.hpp | 6 +++--- cpp/src/utilities/prefetch.cpp | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cpp/include/cudf/utilities/prefetch.hpp b/cpp/include/cudf/utilities/prefetch.hpp index d794ef7d015..3384181fc37 100644 --- a/cpp/include/cudf/utilities/prefetch.hpp +++ b/cpp/include/cudf/utilities/prefetch.hpp @@ -21,7 +21,7 @@ #include #include -#include +#include #include #include @@ -53,7 +53,7 @@ class prefetch_config { * @param key The configuration key. * @return The value of the configuration key. */ - bool get(std::string_view key) const; + bool get(std::string_view key); /** * @brief Set the value of a configuration key. * @@ -73,7 +73,7 @@ class prefetch_config { private: prefetch_config() = default; //< Private constructor to enforce singleton pattern std::map config_values; //< Map of configuration keys to values - std::mutex config_mtx; //< Mutex for thread-safe config access + std::shared_mutex config_mtx; //< Mutex for thread-safe config access }; /** diff --git a/cpp/src/utilities/prefetch.cpp b/cpp/src/utilities/prefetch.cpp index bf803e788a1..9f98782d47c 100644 --- a/cpp/src/utilities/prefetch.cpp +++ b/cpp/src/utilities/prefetch.cpp @@ -32,17 +32,18 @@ prefetch_config& prefetch_config::instance() return instance; } -bool prefetch_config::get(std::string_view key) const +bool prefetch_config::get(std::string_view key) { - std::scoped_lock lock(config_mtx); - // Default to not prefetching - if (config_values.find(key.data()) == config_values.end()) { return false; } + std::shared_lock lock(config_mtx); + if (config_values.find(key.data()) == config_values.end()) { + return false; // default to not prefetching + } return config_values.at(key.data()); } void prefetch_config::set(std::string_view key, bool value) { - std::scoped_lock lock(config_mtx); + std::lock_guard lock(config_mtx); config_values[key.data()] = value; } From 080a136a3f9c08e845f67e1b9b293666a6d74f3f Mon Sep 17 00:00:00 2001 From: Nghia Truong Date: Mon, 29 Jul 2024 12:47:40 -0700 Subject: [PATCH 5/5] Avoid double lookup Signed-off-by: Nghia Truong --- cpp/src/utilities/prefetch.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/src/utilities/prefetch.cpp b/cpp/src/utilities/prefetch.cpp index 9f98782d47c..86d6cc00764 100644 --- a/cpp/src/utilities/prefetch.cpp +++ b/cpp/src/utilities/prefetch.cpp @@ -35,10 +35,8 @@ prefetch_config& prefetch_config::instance() bool prefetch_config::get(std::string_view key) { std::shared_lock lock(config_mtx); - if (config_values.find(key.data()) == config_values.end()) { - return false; // default to not prefetching - } - return config_values.at(key.data()); + auto const it = config_values.find(key.data()); + return it == config_values.end() ? false : it->second; // default to not prefetching } void prefetch_config::set(std::string_view key, bool value)