Merge pull request #4413 from danpoe/refactor/sharing-map

Sharing map refactorings

Author: danpoe

src/util/sharing_map.h

@@ -350,7 +350,7 @@ class sharing_mapt
 
   std::size_t count_unmarked_nodes(
     bool leafs_only,
-    std::set<void *> &marked,
+    std::set<const void *> &marked,
     bool mark = true) const;
 
   // dummy element returned when no element was found
@@ -423,8 +423,10 @@ ::iterate(
 }
 
 SHARING_MAPT(std::size_t)
-::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
-  const
+::count_unmarked_nodes(
+  bool leafs_only,
+  std::set<const void *> &marked,
+  bool mark) const
 {
   if(empty())
     return 0;
@@ -447,8 +449,10 @@ ::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
 
     // internal node or container node
     const innert *ip = static_cast<const innert *>(bp);
-    const unsigned use_count = ip->data.use_count();
-    void *raw_ptr = ip->data.get();
+    const unsigned use_count = ip->use_count();
+    const void *raw_ptr = ip->is_internal()
+                            ? (const void *)&ip->read_internal()
+                            : (const void *)&ip->read_container();
 
     if(use_count >= 2)
     {
@@ -488,8 +492,8 @@ ::count_unmarked_nodes(bool leafs_only, std::set<void *> &marked, bool mark)
 
       for(const auto &l : ll)
       {
-        const unsigned leaf_use_count = l.data.use_count();
-        void *leaf_raw_ptr = l.data.get();
+        const unsigned leaf_use_count = l.use_count();
+        const void *leaf_raw_ptr = &l.read();
 
         if(leaf_use_count >= 2)
         {
@@ -528,7 +532,7 @@ ::get_sharing_stats(
   Iterator end,
   std::function<sharing_mapt &(const Iterator)> f)
 {
-  std::set<void *> marked;
+  std::set<const void *> marked;
   sharing_map_statst sms;
 
   // We do a separate pass over the tree for each statistic. This is not very

src/util/sharing_node.h

@@ -105,6 +105,9 @@ class sharing_node_baset
 SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
 {
 public:
+  typedef small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS> datat;
+  typedef typename datat::use_countt use_countt;
+
   typedef d_internalt<SN_TYPE_ARGS> d_it;
   typedef d_containert<SN_TYPE_ARGS> d_ct;
 
@@ -113,18 +116,18 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
   typedef typename d_ct::leaft leaft;
   typedef typename d_ct::leaf_listt leaf_listt;
 
-  sharing_node_innert() : data(empty_data)
+  sharing_node_innert()
   {
   }
 
   bool empty() const
   {
-    return data == empty_data;
+    return !data;
   }
 
   void clear()
   {
-    data = empty_data;
+    data.reset();
   }
 
   bool shares_with(const sharing_node_innert &other) const
@@ -134,6 +137,11 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
     return data == other.data;
   }
 
+  use_countt use_count() const
+  {
+    return data.use_count();
+  }
+
   void swap(sharing_node_innert &other)
   {
     data.swap(other.data);
@@ -151,45 +159,13 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
     return data.is_derived_v();
   }
 
-  d_it &write_internal()
-  {
-    if(data == empty_data)
-    {
-      data = make_shared_derived_u<SN_PTR_TYPE_ARGS>();
-    }
-    else if(data.use_count() > 1)
-    {
-      data = make_shared_derived_u<SN_PTR_TYPE_ARGS>(*data.get_derived_u());
-    }
-
-    SN_ASSERT(data.use_count() == 1);
-
-    return *data.get_derived_u();
-  }
-
   const d_it &read_internal() const
   {
     SN_ASSERT(!empty());
 
     return *data.get_derived_u();
   }
 
-  d_ct &write_container()
-  {
-    if(data == empty_data)
-    {
-      data = make_shared_derived_v<SN_PTR_TYPE_ARGS>();
-    }
-    else if(data.use_count() > 1)
-    {
-      data = make_shared_derived_v<SN_PTR_TYPE_ARGS>(*data.get_derived_v());
-    }
-
-    SN_ASSERT(data.use_count() == 1);
-
-    return *data.get_derived_v();
-  }
-
   const d_ct &read_container() const
   {
     SN_ASSERT(!empty());
@@ -338,13 +314,41 @@ SN_TYPE_PAR_DEF class sharing_node_innert : public sharing_node_baset
     SN_ASSERT_USE(r, r == 1);
   }
 
-  small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS> data;
-  static small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS> empty_data;
-};
+protected:
+  d_it &write_internal()
+  {
+    if(!data)
+    {
+      data = make_shared_derived_u<SN_PTR_TYPE_ARGS>();
+    }
+    else if(data.use_count() > 1)
+    {
+      data = make_shared_derived_u<SN_PTR_TYPE_ARGS>(*data.get_derived_u());
+    }
 
-SN_TYPE_PAR_DEF small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS>
-  sharing_node_innert<SN_TYPE_ARGS>::empty_data =
-    small_shared_two_way_ptrt<SN_PTR_TYPE_ARGS>();
+    SN_ASSERT(data.use_count() == 1);
+
+    return *data.get_derived_u();
+  }
+
+  d_ct &write_container()
+  {
+    if(!data)
+    {
+      data = make_shared_derived_v<SN_PTR_TYPE_ARGS>();
+    }
+    else if(data.use_count() > 1)
+    {
+      data = make_shared_derived_v<SN_PTR_TYPE_ARGS>(*data.get_derived_v());
+    }
+
+    SN_ASSERT(data.use_count() == 1);
+
+    return *data.get_derived_v();
+  }
+
+  datat data;
+};
 
 // Leafs
 
@@ -362,9 +366,12 @@ SN_TYPE_PAR_DECL class d_leaft : public small_shared_pointeet<unsigned>
 SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
 {
 public:
+  typedef small_shared_ptrt<SN_PTR_TYPE_ARG> datat;
+  typedef decltype(datat().use_count()) use_countt;
+
   typedef d_leaft<SN_TYPE_ARGS> d_lt;
 
-  sharing_node_leaft(const keyT &k, const valueT &v) : data(empty_data)
+  sharing_node_leaft(const keyT &k, const valueT &v)
   {
     SN_ASSERT(empty());
 
@@ -384,45 +391,27 @@ SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
 
   bool empty() const
   {
-    return data == empty_data;
+    return !data;
   }
 
   void clear()
   {
-    data = empty_data;
+    data.reset();
   }
 
   bool shares_with(const sharing_node_leaft &other) const
   {
     return data == other.data;
   }
 
-  void swap(sharing_node_leaft &other)
+  use_countt use_count() const
   {
-    data.swap(other.data);
-  }
-
-  d_lt &write()
-  {
-    SN_ASSERT(data.use_count() > 0);
-
-    if(data == empty_data)
-    {
-      data = make_small_shared_ptr<d_lt>();
-    }
-    else if(data.use_count() > 1)
-    {
-      data = make_small_shared_ptr<d_lt>(*data);
-    }
-
-    SN_ASSERT(data.use_count() == 1);
-
-    return *data;
+    return data.use_count();
   }
 
-  const d_lt &read() const
+  void swap(sharing_node_leaft &other)
   {
-    return *data;
+    data.swap(other.data);
   }
 
   // Accessors
@@ -452,12 +441,29 @@ SN_TYPE_PAR_DEF class sharing_node_leaft : public sharing_node_baset
     return write().v;
   }
 
-  small_shared_ptrt<SN_PTR_TYPE_ARG> data;
-  static small_shared_ptrt<SN_PTR_TYPE_ARG> empty_data;
-};
+  const d_lt &read() const
+  {
+    return *data;
+  }
 
-SN_TYPE_PAR_DEF small_shared_ptrt<SN_PTR_TYPE_ARG>
-  sharing_node_leaft<SN_TYPE_ARGS>::empty_data =
-    make_small_shared_ptr<SN_PTR_TYPE_ARG>();
+protected:
+  d_lt &write()
+  {
+    if(!data)
+    {
+      data = make_small_shared_ptr<d_lt>();
+    }
+    else if(data.use_count() > 1)
+    {
+      data = make_small_shared_ptr<d_lt>(*data);
+    }
+
+    SN_ASSERT(data.use_count() == 1);
+
+    return *data;
+  }
+
+  datat data;
+};
 
 #endif

src/util/small_shared_two_way_ptr.h

@@ -101,30 +101,13 @@ class small_shared_two_way_ptrt final
 
   ~small_shared_two_way_ptrt()
   {
-    if(!p)
-    {
-      return;
-    }
-
-    auto use_count = p->use_count();
+    destruct();
+  }
 
-    if(use_count == 1)
-    {
-      if(p->is_derived_u())
-      {
-        U *u = static_cast<U *>(p);
-        delete u;
-      }
-      else
-      {
-        V *v = static_cast<V *>(p);
-        delete v;
-      }
-    }
-    else
-    {
-      p->decrement_use_count();
-    }
+  void reset()
+  {
+    destruct();
+    p = nullptr;
   }
 
   void swap(small_shared_two_way_ptrt &rhs)
@@ -186,6 +169,34 @@ class small_shared_two_way_ptrt final
   }
 
 private:
+  void destruct()
+  {
+    if(!p)
+    {
+      return;
+    }
+
+    auto use_count = p->use_count();
+
+    if(use_count == 1)
+    {
+      if(p->is_derived_u())
+      {
+        U *u = static_cast<U *>(p);
+        delete u;
+      }
+      else
+      {
+        V *v = static_cast<V *>(p);
+        delete v;
+      }
+    }
+    else
+    {
+      p->decrement_use_count();
+    }
+  }
+
   pointeet *p = nullptr;
 };
 

unit/util/sharing_map.cpp

@@ -385,7 +385,7 @@ void sharing_map_sharing_stats_test()
 {
   SECTION("count nodes")
   {
-    std::set<void *> marked;
+    std::set<const void *> marked;
     smt sm;
     int count = 0;
 
@@ -410,7 +410,7 @@ void sharing_map_sharing_stats_test()
 
   SECTION("marking")
   {
-    std::set<void *> marked;
+    std::set<const void *> marked;
     smt sm;
 
     fill(sm);