Improvements to glz::async_map (#1465)

stephenberry · web-flow · commit 8ef109eed85f · 2024-12-03T15:25:41.000-06:00
* Adding operator[] to async_map and simplifying map_subscriptable constraint

* Add operator* for value_proxy

* proxy_value now returns V&amp;

* serialization and deserialization for async_map
diff --git a/include/glaze/concepts/container_concepts.hpp b/include/glaze/concepts/container_concepts.hpp
@@ -155,7 +155,7 @@ namespace glz::detail
    concept map_subscriptable = requires(T container) {
       {
          container[std::declval<typename T::key_type>()]
-      } -> std::same_as<typename T::mapped_type&>;
+      };
    };
 
    template <typename T>
diff --git a/include/glaze/thread/async_map.hpp b/include/glaze/thread/async_map.hpp
@@ -14,9 +14,6 @@
 #include <utility>
 #include <vector>
 
-#include "glaze/core/common.hpp"
-#include "glaze/util/type_traits.hpp"
-
 // This async_map is intended to hold thread safe value types (V)
 
 namespace glz
@@ -138,22 +135,22 @@ namespace glz
          pointer operator->() const { return (*item_it).get(); }
 
          // Equality Comparison
-         bool operator==(const iterator& other) const { return item_it == other.item_it; }
+         bool operator==(const iterator& other) const noexcept { return item_it == other.item_it; }
 
          // Inequality Comparison
-         bool operator!=(const iterator& other) const { return !(*this == other); }
+         bool operator!=(const iterator& other) const noexcept { return !(*this == other); }
 
          // Comparison operators with iterator
-         bool operator<(const iterator& other) const { return item_it < other.item_it; }
-         bool operator>(const iterator& other) const { return item_it > other.item_it; }
-         bool operator<=(const iterator& other) const { return item_it <= other.item_it; }
+         bool operator<(const iterator& other) const noexcept { return item_it < other.item_it; }
+         bool operator>(const iterator& other) const noexcept { return item_it > other.item_it; }
+         bool operator<=(const iterator& other) const noexcept { return item_it <= other.item_it; }
          bool operator>=(const iterator& other) const { return item_it >= other.item_it; }
 
          // Comparison operators with const_iterator
-         bool operator<(const const_iterator& other) const { return item_it < other.item_it; }
-         bool operator>(const const_iterator& other) const { return item_it > other.item_it; }
-         bool operator<=(const const_iterator& other) const { return item_it <= other.item_it; }
-         bool operator>=(const const_iterator& other) const { return item_it >= other.item_it; }
+         bool operator<(const const_iterator& other) const noexcept { return item_it < other.item_it; }
+         bool operator>(const const_iterator& other) const noexcept { return item_it > other.item_it; }
+         bool operator<=(const const_iterator& other) const noexcept { return item_it <= other.item_it; }
+         bool operator>=(const const_iterator& other) const noexcept { return item_it >= other.item_it; }
       };
 
       class const_iterator
@@ -223,41 +220,43 @@ namespace glz
          pointer operator->() const { return (*item_it).get(); }
 
          // Equality Comparison
-         bool operator==(const const_iterator& other) const { return item_it == other.item_it; }
+         bool operator==(const const_iterator& other) const noexcept { return item_it == other.item_it; }
 
          // Inequality Comparison
-         bool operator!=(const const_iterator& other) const { return !(*this == other); }
+         bool operator!=(const const_iterator& other) const noexcept { return !(*this == other); }
 
          // Comparison operators with const_iterator
-         bool operator<(const const_iterator& other) const { return item_it < other.item_it; }
-         bool operator>(const const_iterator& other) const { return item_it > other.item_it; }
-         bool operator<=(const const_iterator& other) const { return item_it <= other.item_it; }
-         bool operator>=(const const_iterator& other) const { return item_it >= other.item_it; }
+         bool operator<(const const_iterator& other) const noexcept { return item_it < other.item_it; }
+         bool operator>(const const_iterator& other) const noexcept { return item_it > other.item_it; }
+         bool operator<=(const const_iterator& other) const noexcept { return item_it <= other.item_it; }
+         bool operator>=(const const_iterator& other) const noexcept { return item_it >= other.item_it; }
 
          // Comparison operators with iterator
-         bool operator<(const iterator& other) const { return item_it < other.item_it; }
-         bool operator>(const iterator& other) const { return item_it > other.item_it; }
-         bool operator<=(const iterator& other) const { return item_it <= other.item_it; }
-         bool operator>=(const iterator& other) const { return item_it >= other.item_it; }
+         bool operator<(const iterator& other) const noexcept { return item_it < other.item_it; }
+         bool operator>(const iterator& other) const noexcept { return item_it > other.item_it; }
+         bool operator<=(const iterator& other) const noexcept { return item_it <= other.item_it; }
+         bool operator>=(const iterator& other) const noexcept { return item_it >= other.item_it; }
       };
 
       // Value Proxy Class Definition
       class value_proxy
       {
         private:
-         value_type& value_ref;
+         V& value_ref;
          std::shared_ptr<std::shared_lock<std::shared_mutex>> shared_lock_ptr;
          std::shared_ptr<std::unique_lock<std::shared_mutex>> unique_lock_ptr;
 
         public:
-         value_proxy(value_type& value_ref,
+         value_proxy(V& value_ref,
                      std::shared_ptr<std::shared_lock<std::shared_mutex>> existing_shared_lock = nullptr,
                      std::shared_ptr<std::unique_lock<std::shared_mutex>> existing_unique_lock = nullptr)
             : value_ref(value_ref), shared_lock_ptr(existing_shared_lock), unique_lock_ptr(existing_unique_lock)
          {
             // Ensure that a lock is provided
             assert(shared_lock_ptr || unique_lock_ptr);
          }
+         
+         static constexpr bool glaze_value_proxy = true;
 
          // Disable Copy and Move
          value_proxy(const value_proxy&) = delete;
@@ -266,15 +265,29 @@ namespace glz
          value_proxy& operator=(value_proxy&&) = delete;
 
          // Access the value
-         V& value() { return value_ref.second; }
+         V& value() { return value_ref; }
 
-         const V& value() const { return value_ref.second; }
+         const V& value() const { return value_ref; }
 
          // Arrow Operator
-         value_type* operator->() { return &value_ref; }
+         V* operator->() { return &value_ref; }
+         
+         const V* operator->() const { return &value_ref; }
+         
+         V& operator*() { return value_ref; }
+         
+         const V& operator*() const { return value_ref; }
 
          // Implicit Conversion to V&
-         operator V&() { return value_ref.second; }
+         operator V&() { return value_ref; }
+         
+         operator const V&() const { return value_ref; }
+         
+         template <class T>
+         value_proxy& operator=(const T& other) {
+            value_ref = other;
+            return *this;
+         }
 
          bool operator==(const V& other) const { return value() == other; }
       };
@@ -283,11 +296,11 @@ namespace glz
       class const_value_proxy
       {
         private:
-         const_value_type& value_ref;
+         const V& value_ref;
          std::shared_ptr<std::shared_lock<std::shared_mutex>> shared_lock_ptr;
 
         public:
-         const_value_proxy(const_value_type& value_ref,
+         const_value_proxy(const V& value_ref,
                            std::shared_ptr<std::shared_lock<std::shared_mutex>> existing_shared_lock)
             : value_ref(value_ref), shared_lock_ptr(existing_shared_lock)
          {
@@ -302,24 +315,60 @@ namespace glz
          const_value_proxy& operator=(const_value_proxy&&) = delete;
 
          // Access the value
-         const V& value() const { return value_ref.second; }
+         const V& value() const { return value_ref; }
 
          // Arrow Operator
-         const_value_type* operator->() const { return &value_ref; }
+         const V* operator->() const { return &value_ref; }
+         
+         const V& operator*() const { return value_ref; }
 
          // Implicit Conversion to const V&
-         operator const V&() const { return value_ref.second; }
+         operator const V&() const { return value_ref; }
 
          bool operator==(const V& other) const { return value() == other; }
       };
-
-      template <class KeyType>
-      [[deprecated(
-         "operator[] is not allowed with async_map because it would require expensive unique locks")]] value_proxy
-      operator[](const KeyType&)
+      
+      value_proxy operator[](const K& key)
       {
-         static_assert(false_v<KeyType>);
-      };
+         // Acquire a shared lock to search for the key
+         std::shared_lock<std::shared_mutex> shared_lock(mutex);
+         auto [it, found] = binary_search_key(key);
+
+         if (found) {
+            auto shared_lock_ptr = std::make_shared<std::shared_lock<std::shared_mutex>>(std::move(shared_lock));
+            return value_proxy((*it)->second, shared_lock_ptr);
+         }
+         else {
+            // Key doesn't exist; release the shared_lock
+            shared_lock.unlock();
+            // Acquire a unique lock to modify the map
+            std::unique_lock<std::shared_mutex> unique_lock(mutex);
+
+            // Double-check if the key was inserted by another thread
+            std::tie(it, found) = binary_search_key(key);
+
+            if (!found) {
+               // Insert a new element with default-constructed value
+               it = items.insert(it, std::make_unique<std::pair<K, V>>(
+                                               std::piecewise_construct,
+                                               std::forward_as_tuple(key),
+                                               std::forward_as_tuple()));
+            }
+
+            unique_lock.unlock();
+            shared_lock.lock();
+            
+            // Find the value once again in case a modification occurred
+            std::tie(it, found) = binary_search_key(key);
+            
+            if (!found) {
+               throw std::out_of_range("Key was removed by another thread");
+            }
+
+            auto shared_lock_ptr = std::make_shared<std::shared_lock<std::shared_mutex>>(std::move(shared_lock));
+            return value_proxy((*it)->second, shared_lock_ptr);
+         }
+      }
 
       // Insert method behaves like std::map::insert
       std::pair<iterator, bool> insert(const std::pair<K, V>& pair)
@@ -447,7 +496,7 @@ namespace glz
          auto [it, found] = binary_search_key(key);
 
          if (found) {
-            return value_proxy(*(*it), shared_lock_ptr);
+            return value_proxy((*it)->second, shared_lock_ptr);
          }
          else {
             throw std::out_of_range("Key not found");
@@ -463,7 +512,7 @@ namespace glz
          auto [it, found] = binary_search_key(key);
 
          if (found) {
-            return const_value_proxy(*(*it), shared_lock_ptr);
+            return const_value_proxy((*it)->second, shared_lock_ptr);
          }
          else {
             throw std::out_of_range("Key not found");
@@ -521,3 +570,19 @@ namespace glz
       }
    };
 }
+
+namespace glz::detail
+{
+   template <class T>
+   concept is_value_proxy = requires { T::glaze_value_proxy; };
+   
+   template <is_value_proxy T>
+   struct from<JSON, T>
+   {
+      template <auto Opts>
+      static void op(auto&& value, is_context auto&& ctx, auto&& it, auto&& end)
+      {
+         read<JSON>::op<Opts>(value.value(), ctx, it, end);
+      }
+   };
+}
diff --git a/tests/exceptions_test/exceptions_test.cpp b/tests/exceptions_test/exceptions_test.cpp
@@ -250,6 +250,23 @@ suite async_map_tests = [] {
          std::cout << it->second << '\n';
       }
    };
+   
+   "async_map write_json"_test = [] {
+      static_assert(glz::detail::readable_map_t<glz::async_map<std::string, std::atomic<int>>>);
+      
+      glz::async_map<std::string, std::atomic<int>> map;
+      map["one"] = 1;
+      map["two"] = 2;
+      
+      std::string buffer{};
+      expect(not glz::write_json(map, buffer));
+      expect(buffer == R"({"one":1,"two":2})") << buffer;
+      
+      map.clear();
+      expect(not glz::read_json(map, buffer));
+      expect(map.at("one").value() == 1);
+      expect(map.at("two").value() == 2);
+   };
 };
 
 int main() { return 0; }

Original file line number	Diff line number	Diff line change
`@@ -155,7 +155,7 @@ namespace glz::detail`
`155`	`155`	`concept map_subscriptable = requires(T container) {`
`156`	`156`	`{`
`157`	`157`	`container[std::declval<typename T::key_type>()]`
`158`		`- } -> std::same_as<typename T::mapped_type&>;`
	`158`	`+ };`
`159`	`159`	`};`
`160`	`160`
`161`	`161`	`template <typename T>`