multithreading: Add AtomicValue to atomically set/get value

... and use it to replace the commonly used pattern in
Expression, LinkedMap, and GitTreeEntry. Furthermore, remove
assignment operators for Expression and LinkedMap as those
are considered to be used in an immutable manner anyway.

9 files changed, 88 insertions, 95 deletions

diff --git a/src/buildtool/build_engine/expression/TARGETS b/src/buildtool/build_engine/expression/TARGETS
index 93d3362c..d6adc88c 100644
--- a/src/buildtool/build_engine/expression/TARGETS
+++ b/src/buildtool/build_engine/expression/TARGETS
@@ -4,8 +4,8 @@
   , "hdrs": ["linked_map.hpp"]
   , "deps":
     [ ["@", "fmt", "", "fmt"]
+    , ["src/buildtool/multithreading", "atomic_value"]
     , ["src/utils/cpp", "hash_combine"]
-    , ["src/utils/cpp", "atomic"]
     ]
   , "stage": ["src", "buildtool", "build_engine", "expression"]
   }
@@ -44,12 +44,12 @@
     , ["src/buildtool/common", "artifact_description"]
     , ["src/buildtool/crypto", "hash_generator"]
     , ["src/buildtool/logging", "logging"]
+    , ["src/buildtool/multithreading", "atomic_value"]
     , ["src/utils/cpp", "type_safe_arithmetic"]
     , ["src/utils/cpp", "json"]
     , ["src/utils/cpp", "hash_combine"]
     , ["src/utils/cpp", "hex_string"]
     , ["src/utils/cpp", "concepts"]
-    , ["src/utils/cpp", "atomic"]
     , ["src/utils/cpp", "path"]
     , ["@", "gsl-lite", "", "gsl-lite"]
     ]
diff --git a/src/buildtool/build_engine/expression/expression.cpp b/src/buildtool/build_engine/expression/expression.cpp
index 679ebcbd..763f51f8 100644
--- a/src/buildtool/build_engine/expression/expression.cpp
+++ b/src/buildtool/build_engine/expression/expression.cpp
@@ -149,16 +149,7 @@ auto Expression::ToString() const -> std::string {
 
 // NOLINTNEXTLINE(misc-no-recursion)
 auto Expression::ToHash() const noexcept -> std::string {
-    if (hash_.load() == nullptr) {
-        if (not hash_loading_.exchange(true)) {
-            hash_ = std::make_shared<std::string>(ComputeHash());
-            hash_.notify_all();
-        }
-        else {
-            hash_.wait(nullptr);
-        }
-    }
-    return *hash_.load();
+    return hash_.SetOnceAndGet([this] { return ComputeHash(); });
 }
 
 // NOLINTNEXTLINE(misc-no-recursion)
diff --git a/src/buildtool/build_engine/expression/expression.hpp b/src/buildtool/build_engine/expression/expression.hpp
index 412de057..6acb1e0e 100644
--- a/src/buildtool/build_engine/expression/expression.hpp
+++ b/src/buildtool/build_engine/expression/expression.hpp
@@ -20,7 +20,7 @@
 #include "src/buildtool/build_engine/expression/target_result.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
 #include "src/buildtool/crypto/hash_generator.hpp"
-#include "src/utils/cpp/atomic.hpp"
+#include "src/buildtool/multithreading/atomic_value.hpp"
 #include "src/utils/cpp/hex_string.hpp"
 #include "src/utils/cpp/json.hpp"
 
@@ -63,22 +63,10 @@ class Expression {
 
     Expression() noexcept = default;
     ~Expression() noexcept = default;
-    Expression(Expression const& other) noexcept
-        : data_{other.data_}, hash_{other.hash_.load()} {}
-    Expression(Expression&& other) noexcept
-        : data_{std::move(other.data_)}, hash_{other.hash_.load()} {}
-    auto operator=(Expression const& other) noexcept -> Expression& {
-        if (this != &other) {
-            data_ = other.data_;
-        }
-        hash_ = other.hash_.load();
-        return *this;
-    }
-    auto operator=(Expression&& other) noexcept -> Expression& {
-        data_ = std::move(other.data_);
-        hash_ = other.hash_.load();
-        return *this;
-    }
+    Expression(Expression const& other) noexcept = default;
+    Expression(Expression&& other) noexcept = default;
+    auto operator=(Expression const& other) noexcept = delete;
+    auto operator=(Expression&& other) noexcept = delete;
 
     template <class T>
     requires(IsValidType<std::remove_cvref_t<T>>())
@@ -256,8 +244,7 @@ class Expression {
                  map_t>
         data_{none_t{}};
 
-    mutable atomic_shared_ptr<std::string> hash_{};
-    mutable std::atomic<bool> hash_loading_{};
+    AtomicValue<std::string> hash_{};
 
     template <class T, std::size_t kIndex = 0>
     requires(IsValidType<T>()) [[nodiscard]] static consteval auto GetIndexOf()
diff --git a/src/buildtool/build_engine/expression/linked_map.hpp b/src/buildtool/build_engine/expression/linked_map.hpp
index 5c6da558..6266c2ce 100644
--- a/src/buildtool/build_engine/expression/linked_map.hpp
+++ b/src/buildtool/build_engine/expression/linked_map.hpp
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "fmt/core.h"
+#include "src/buildtool/multithreading/atomic_value.hpp"
 #include "src/utils/cpp/atomic.hpp"
 #include "src/utils/cpp/hash_combine.hpp"
 
@@ -110,32 +111,12 @@ class LinkedMap {
     }
 
     LinkedMap() noexcept = default;
-    LinkedMap(LinkedMap const& other) noexcept
-        : next_{other.next_},
-          content_{other.content_},
-          map_{other.map_},
-          items_{other.items_.load()} {}
-    LinkedMap(LinkedMap&& other) noexcept
-        : next_{std::move(other.next_)},
-          content_{std::move(other.content_)},
-          map_{std::move(other.map_)},
-          items_{other.items_.load()} {}
+    LinkedMap(LinkedMap const& other) noexcept = default;
+    LinkedMap(LinkedMap&& other) noexcept = default;
     ~LinkedMap() noexcept = default;
 
-    auto operator=(LinkedMap const& other) noexcept -> LinkedMap& {
-        next_ = other.next_;
-        content_ = other.content_;
-        map_ = other.map_;
-        items_ = other.items_.load();
-        return *this;
-    }
-    auto operator=(LinkedMap&& other) noexcept -> LinkedMap& {
-        next_ = std::move(other.next_);
-        content_ = std::move(other.content_);
-        map_ = std::move(other.map_);
-        items_ = other.items_.load();
-        return *this;
-    }
+    auto operator=(LinkedMap const& other) noexcept = delete;
+    auto operator=(LinkedMap&& other) noexcept = delete;
 
     [[nodiscard]] auto contains(K const& key) const noexcept -> bool {
         return static_cast<bool>(Find(key));
@@ -276,23 +257,10 @@ class LinkedMap {
 
     // NOTE: Expensive, needs to compute sorted items.
     [[nodiscard]] auto Items() const& -> items_t const& {
-        if (items_.load() == nullptr) {
-            if (not items_loading_.exchange(true)) {
-                items_ = std::make_shared<items_t>(ComputeSortedItems());
-                items_.notify_all();
-            }
-            else {
-                items_.wait(nullptr);
-            }
-        }
-        return *items_.load();
+        return items_.SetOnceAndGet([this] { return ComputeSortedItems(); });
     }
 
-    // NOTE: Expensive, needs to compute sorted items.
-    [[nodiscard]] auto Items() && -> items_t {
-        return items_.load() == nullptr ? ComputeSortedItems()
-                                        : std::move(*items_.load());
-    }
+    [[nodiscard]] auto Items() && = delete;
 
     // NOTE: Expensive, needs to compute sorted items.
     [[nodiscard]] auto Keys() const -> keys_t {
@@ -323,8 +291,7 @@ class LinkedMap {
     Ptr content_{};           // content of this map if set
     underlying_map_t map_{};  // content of this map if content_ is not set
 
-    mutable atomic_shared_ptr<items_t> items_{};
-    mutable std::atomic<bool> items_loading_{};
+    AtomicValue<items_t> items_{};
 
     [[nodiscard]] auto ComputeSortedItems() const noexcept -> items_t {
         auto size =
diff --git a/src/buildtool/file_system/TARGETS b/src/buildtool/file_system/TARGETS
index 349c6d03..228cc3c9 100644
--- a/src/buildtool/file_system/TARGETS
+++ b/src/buildtool/file_system/TARGETS
@@ -48,7 +48,7 @@
     , "object_type"
     , "file_system_manager"
     , ["src/buildtool/logging", "logging"]
-    , ["src/utils/cpp", "atomic"]
+    , ["src/buildtool/multithreading", "atomic_value"]
     , ["src/utils/cpp", "hex_string"]
     , ["@", "gsl-lite", "", "gsl-lite"]
     ]
diff --git a/src/buildtool/file_system/git_tree.cpp b/src/buildtool/file_system/git_tree.cpp
index 98281707..f4912a7b 100644
--- a/src/buildtool/file_system/git_tree.cpp
+++ b/src/buildtool/file_system/git_tree.cpp
@@ -13,9 +13,6 @@ namespace {
 
 constexpr auto kOIDRawSize{GIT_OID_RAWSZ};
 
-auto const kLoadTreeError =
-    std::make_shared<std::optional<GitTree>>(std::nullopt);
-
 [[nodiscard]] auto PermToType(std::string const& perm_str) noexcept
     -> std::optional<ObjectType> {
     constexpr auto kPermBase = 8;
@@ -154,26 +151,17 @@ auto GitTreeEntry::Blob() const noexcept -> std::optional<std::string> {
 }
 
 auto GitTreeEntry::Tree() const& noexcept -> std::optional<GitTree> const& {
-    auto ptr = tree_cached_.load();
-    if (not ptr) {
-        if (not tree_loading_.exchange(true)) {
-            ptr = kLoadTreeError;
-            std::optional<std::string> obj{};
-            if (IsTree() and (obj = cas_->ReadObject(raw_id_))) {
+    return tree_cached_.SetOnceAndGet([this]() -> std::optional<GitTree> {
+        std::optional<std::string> obj{};
+        if (IsTree()) {
+            if (auto obj = cas_->ReadObject(raw_id_)) {
                 if (auto entries = ParseRawTreeObject(cas_, *obj)) {
-                    ptr = std::make_shared<std::optional<GitTree>>(
-                        GitTree{cas_, std::move(*entries), raw_id_});
+                    return GitTree{cas_, std::move(*entries), raw_id_};
                 }
             }
-            tree_cached_.store(ptr);
-            tree_cached_.notify_all();
-        }
-        else {
-            tree_cached_.wait(nullptr);
-            ptr = tree_cached_.load();
         }
-    }
-    return *ptr;
+        return std::nullopt;
+    });
 }
 
 auto GitTreeEntry::Size() const noexcept -> std::optional<std::size_t> {
diff --git a/src/buildtool/file_system/git_tree.hpp b/src/buildtool/file_system/git_tree.hpp
index 8371d785..ff7dc828 100644
--- a/src/buildtool/file_system/git_tree.hpp
+++ b/src/buildtool/file_system/git_tree.hpp
@@ -8,7 +8,7 @@
 #include "gsl-lite/gsl-lite.hpp"
 #include "src/buildtool/file_system/git_cas.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
-#include "src/utils/cpp/atomic.hpp"
+#include "src/buildtool/multithreading/atomic_value.hpp"
 #include "src/utils/cpp/hex_string.hpp"
 
 class GitTreeEntry;
@@ -85,8 +85,7 @@ class GitTreeEntry {
     gsl::not_null<GitCASPtr> cas_;
     std::string raw_id_;
     ObjectType type_;
-    mutable atomic_shared_ptr<std::optional<GitTree>> tree_cached_{nullptr};
-    mutable std::atomic<bool> tree_loading_{false};
+    AtomicValue<std::optional<GitTree>> tree_cached_{};
 };
 
 using GitTreePtr = std::shared_ptr<GitTree const>;
diff --git a/src/buildtool/multithreading/TARGETS b/src/buildtool/multithreading/TARGETS
index 88bed701..c1545f0f 100644
--- a/src/buildtool/multithreading/TARGETS
+++ b/src/buildtool/multithreading/TARGETS
@@ -52,4 +52,11 @@
     ]
   , "stage": ["src", "buildtool", "multithreading"]
   }
+, "atomic_value":
+  { "type": ["@", "rules", "CC", "library"]
+  , "name": ["atomic_value"]
+  , "hdrs": ["atomic_value.hpp"]
+  , "deps": [["src/utils/cpp", "atomic"]]
+  , "stage": ["src", "buildtool", "multithreading"]
+  }
 }
diff --git a/src/buildtool/multithreading/atomic_value.hpp b/src/buildtool/multithreading/atomic_value.hpp
new file mode 100644
index 00000000..2c108a96
--- /dev/null
+++ b/src/buildtool/multithreading/atomic_value.hpp
@@ -0,0 +1,54 @@
+#ifndef INCLUDED_SRC_BUILDTOOL_MULTITHREADING_ATOMIC_VALUE_HPP
+#define INCLUDED_SRC_BUILDTOOL_MULTITHREADING_ATOMIC_VALUE_HPP
+
+#include <atomic>
+#include <functional>
+#include <optional>
+
+#include "src/utils/cpp/atomic.hpp"
+
+// Value that can be set and get atomically. Reset is not thread-safe.
+template <class T>
+class AtomicValue {
+  public:
+    AtomicValue() noexcept = default;
+    AtomicValue(AtomicValue const& other) noexcept
+        : data_{other.data_.load()} {}
+    AtomicValue(AtomicValue&& other) noexcept : data_{other.data_.load()} {}
+    ~AtomicValue() noexcept = default;
+
+    auto operator=(AtomicValue const& other) noexcept = delete;
+    auto operator=(AtomicValue&& other) noexcept = delete;
+
+    // Atomically set value once and return its reference. If this method is
+    // called multiple times concurrently, the setter is called only once. In
+    // any case, this method blocks until the value is ready.
+    [[nodiscard]] auto SetOnceAndGet(
+        std::function<T()> const& setter) const& noexcept -> T const& {
+        if (data_.load() == nullptr) {
+            if (not load_.exchange(true)) {
+                data_.store(std::make_shared<T>(setter()));
+                data_.notify_all();
+            }
+            else {
+                data_.wait(nullptr);
+            }
+        }
+        return *data_.load();
+    }
+
+    [[nodiscard]] auto SetOnceAndGet(std::function<T()> const& setter) && =
+        delete;
+
+    // Reset, not thread-safe!
+    void Reset() noexcept {
+        load_ = false;
+        data_ = nullptr;
+    }
+
+  private:
+    mutable std::atomic<bool> load_{false};
+    mutable atomic_shared_ptr<T> data_{nullptr};
+};
+
+#endif  // INCLUDED_SRC_BUILDTOOL_MULTITHREADING_ATOMIC_VALUE_HPP