From 90a35644ac96ca07485b25b37c85dfdb67f65442 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 7 Jan 2025 21:38:32 +0000 Subject: [PATCH] thread_safe_memoizer.h: use std::atomic> in c++20 --- .../core/src/util/thread_safe_memoizer.h | 162 +++++++++++++++--- .../unit/util/thread_safe_memoizer_test.cc | 1 - 2 files changed, 139 insertions(+), 24 deletions(-) diff --git a/Firestore/core/src/util/thread_safe_memoizer.h b/Firestore/core/src/util/thread_safe_memoizer.h index 09abc10b73d..299563fb109 100644 --- a/Firestore/core/src/util/thread_safe_memoizer.h +++ b/Firestore/core/src/util/thread_safe_memoizer.h @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2025 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,6 @@ #ifndef FIRESTORE_CORE_SRC_UTIL_THREAD_SAFE_MEMOIZER_H_ #define FIRESTORE_CORE_SRC_UTIL_THREAD_SAFE_MEMOIZER_H_ -#include #include #include @@ -25,6 +24,14 @@ namespace firebase { namespace firestore { namespace util { +// TODO(c++20): Remove the inline namespace once the other #ifdef checks for +// __cpp_lib_atomic_shared_ptr are removed. +#ifdef __cpp_lib_atomic_shared_ptr +inline namespace cpp20_atomic_shared_ptr { +#else +inline namespace cpp11_atomic_free_functions { +#endif + /** * Stores a memoized value in a manner that is safe to be shared between * multiple threads. @@ -32,45 +39,106 @@ namespace util { template class ThreadSafeMemoizer { public: - ThreadSafeMemoizer() = default; + /** + * Creates a new ThreadSafeMemoizer with no memoized value. + */ + ThreadSafeMemoizer() { + memoize_clear(memoized_); + } - ThreadSafeMemoizer(const ThreadSafeMemoizer& other) - : memoized_(std::atomic_load(&other.memoized_)) { + /** + * Copy constructor: creates a new ThreadSafeMemoizer object with the same + * memoized value as the ThreadSafeMemoizer object referred to by the given + * reference. + * + * This ThreadSafeMemoizer object and the given referred-to ThreadSafeMemoizer + * object will share their memoized value with one another. + * + * The runtime performance of this function is O(1). + */ + ThreadSafeMemoizer(const ThreadSafeMemoizer& other) { + operator=(other); } + /** + * Copy assignment operator: replaces this object's memoized value with the + * memoized value of the ThreadSafeMemoizer object referred to by the given + * reference. + * + * This ThreadSafeMemoizer object and the given referred-to ThreadSafeMemoizer + * object will share their memoized value with one another. If this + * ThreadSafeMemoizer object previously shared its memoized value with one or + * more other ThreadSafeMemoizer objects then that relationship is broken. + * + * The runtime performance of this function is O(1). + */ ThreadSafeMemoizer& operator=(const ThreadSafeMemoizer& other) { - std::atomic_store(&memoized_, std::atomic_load(&other.memoized_)); + memoize_store(memoized_, other.memoized_); return *this; } - ThreadSafeMemoizer(ThreadSafeMemoizer&& other) noexcept - : memoized_(std::atomic_load(&other.memoized_)) { + /** + * Move constructor: creates a new ThreadSafeMemoizer object with the same + * memoized value as the ThreadSafeMemoizer object referred to by the given + * reference, also clearing its memoized value. + * + * This ThreadSafeMemoizer object and the given referred-to ThreadSafeMemoizer + * object will NOT share their memoized value with one another; however, this + * ThreadSafeMemoizer object WILL share its memoized value with all other + * ThreadSafeMemoizer objects with which the given ThreadSafeMemoizer object + * formerly shared its memoized value. + * + * The runtime performance of this function is O(1). + */ + ThreadSafeMemoizer(ThreadSafeMemoizer&& other) noexcept { + operator=(std::move(other)); } + /** + * Move assignment operator: replaces this object's memoized value with the + * memoized value of the ThreadSafeMemoizer object referred to by the given + * reference, also clearing its memoized value. + * + * This ThreadSafeMemoizer object and the given referred-to ThreadSafeMemoizer + * object will NOT share their memoized value with one another; however, this + * ThreadSafeMemoizer object WILL share its memoized value with all other + * ThreadSafeMemoizer objects with which the given ThreadSafeMemoizer object + * formerly shared its memoized value. + * + * The runtime performance of this function is O(1). + */ ThreadSafeMemoizer& operator=(ThreadSafeMemoizer&& other) noexcept { - std::atomic_store(&memoized_, std::atomic_load(&other.memoized_)); + memoize_store(memoized_, other.memoized_); + memoize_clear(other.memoized_); return *this; } /** - * Memoize a value. + * Return the memoized value, calculating it with the given function if + * needed. * - * If there is _no_ memoized value then the given function is called to create - * the object to memoize. The created object is then stored for use in future - * invocations as the "memoized value". Finally, a reference to the created - * object is returned. + * If this object _does_ have a memoized value then this function simply + * returns a reference to it and does _not_ call the given function. * - * On the other hand, if there _is_ a memoized value, then a reference to that - * memoized value object is returned and the given function is _not_ called. + * On the other hand, if this object does _not_ have a memoized value then + * the given function is called to calculate the value to memoize. The value + * returned by the function is stored internally as the "memoized value" and + * then returned. * * The given function *must* be idempotent because it _may_ be called more - * than once due to the semantics of std::atomic::compare_exchange_weak(). + * than once due to the semantics of "weak" compare-and-exchange. No reference + * to the given function is retained by this object. The given function will + * be called synchronously by this function, if it is called at all. + * + * This function is thread-safe and may be called concurrently by multiple + * threads. * - * No reference to the given function is retained by this object, and the - * function be called synchronously, if it is called at all. + * The returned reference should only be considered "valid" as long as this + * ThreadSafeMemoizer instance is alive. */ const T& value(const std::function()>& func) { - std::shared_ptr old_memoized = std::atomic_load(&memoized_); + std::shared_ptr old_memoized = memoize_load(memoized_); + while (true) { if (old_memoized) { return *old_memoized; @@ -78,19 +146,67 @@ class ThreadSafeMemoizer { std::shared_ptr new_memoized = func(); - if (std::atomic_compare_exchange_weak(&memoized_, &old_memoized, - new_memoized)) { + if (memoize_compare_exchange(memoized_, old_memoized, new_memoized)) { return *new_memoized; } } } private: + // TODO(c++20): Remove the #ifdef checks for __cpp_lib_atomic_shared_ptr and + // delete all code that is compiled out when __cpp_lib_atomic_shared_ptr is + // defined. +#ifdef __cpp_lib_atomic_shared_ptr + std::atomic> memoized_; + + static void memoize_store(std::atomic>& memoized, + const std::shared_ptr& value) { + memoized.store(value); + } + + static std::shared_ptr memoize_load( + const std::atomic>& memoized) { + return memoized.load(); + } + + static bool memoize_compare_exchange( + std::atomic>& memoized, + std::shared_ptr& expected, + const std::shared_ptr& desired) { + return memoized.compare_exchange_weak(expected, desired); + } + +#else // #ifdef __cpp_lib_atomic_shared_ptr + // NOTE: Always use the std::atomic_XXX() functions to access this shared_ptr + // to ensure thread safety. + // See https://en.cppreference.com/w/cpp/memory/shared_ptr/atomic. std::shared_ptr memoized_; + + static void memoize_store(std::shared_ptr& memoized, + const std::shared_ptr& value) { + std::atomic_store(&memoized, value); + } + + static std::shared_ptr memoize_load(const std::shared_ptr& memoized) { + return std::atomic_load(&memoized); + } + + static bool memoize_compare_exchange(std::shared_ptr& memoized, + std::shared_ptr& expected, + const std::shared_ptr& desired) { + return std::atomic_compare_exchange_weak(&memoized, &expected, desired); + } + +#endif // #ifdef __cpp_lib_atomic_shared_ptr + + static void memoize_clear(std::shared_ptr& memoized) { + memoize_store(memoized, std::shared_ptr()); + } }; +} // namespace cpp20_atomic_shared_ptr/cpp11_atomic_free_functions } // namespace util } // namespace firestore } // namespace firebase -#endif // FIRESTORE_CORE_SRC_UTIL_THREAD_SAFE_MEMOIZER_H_ +#endif // FIRESTORE_CORE_SRC_UTIL_THREAD_SAFE_MEMOIZER_H_ \ No newline at end of file diff --git a/Firestore/core/test/unit/util/thread_safe_memoizer_test.cc b/Firestore/core/test/unit/util/thread_safe_memoizer_test.cc index a4b359d25ed..fa8dc22ce43 100644 --- a/Firestore/core/test/unit/util/thread_safe_memoizer_test.cc +++ b/Firestore/core/test/unit/util/thread_safe_memoizer_test.cc @@ -17,7 +17,6 @@ #include "Firestore/core/src/util/thread_safe_memoizer.h" #include // NOLINT(build/c++11) -#include "absl/memory/memory.h" #include "gtest/gtest.h" namespace {