You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by jm...@apache.org on 2023/01/28 04:44:55 UTC

[datasketches-cpp] branch var_opt_union_allocator created (now 9ba368f)

This is an automated email from the ASF dual-hosted git repository.

jmalkin pushed a change to branch var_opt_union_allocator
in repository https://gitbox.apache.org/repos/asf/datasketches-cpp.git


      at 9ba368f  use allocator properly in varopt union

This branch includes the following new commits:

     new 9ba368f  use allocator properly in varopt union

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org


[datasketches-cpp] 01/01: use allocator properly in varopt union

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmalkin pushed a commit to branch var_opt_union_allocator
in repository https://gitbox.apache.org/repos/asf/datasketches-cpp.git

commit 9ba368fd7090e77d0b853fa2aa70f2ca7dfcaea8
Author: Jon <jm...@apache.org>
AuthorDate: Fri Jan 27 20:44:38 2023 -0800

    use allocator properly in varopt union
---
 sampling/include/var_opt_sketch_impl.hpp | 14 +++++++-------
 sampling/include/var_opt_union.hpp       |  6 +++++-
 sampling/include/var_opt_union_impl.hpp  | 32 ++++++++++++++++++--------------
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/sampling/include/var_opt_sketch_impl.hpp b/sampling/include/var_opt_sketch_impl.hpp
index 2c8db2b..ecbee26 100644
--- a/sampling/include/var_opt_sketch_impl.hpp
+++ b/sampling/include/var_opt_sketch_impl.hpp
@@ -189,16 +189,16 @@ var_opt_sketch<T, A>::~var_opt_sketch() {
       // destroy everything
       const size_t num_to_destroy = std::min(k_ + 1, curr_items_alloc_);
       for (size_t i = 0; i < num_to_destroy; ++i) {
-        allocator_.destroy(data_ + i);
+        data_[i].~T();
       }
     } else {
       // skip gap or anything unused at the end
       for (size_t i = 0; i < h_; ++i) {
-        allocator_.destroy(data_+ i);
+        data_[i].~T();
       }
     
       for (size_t i = h_ + 1; i < h_ + r_ + 1; ++i) {
-        allocator_.destroy(data_ + i);
+        data_[i].~T();
       }
     }
     allocator_.deallocate(data_, curr_items_alloc_);
@@ -658,14 +658,14 @@ void var_opt_sketch<T, A>::reset() {
     // destroy everything
     const size_t num_to_destroy = std::min(k_ + 1, prev_alloc);
     for (size_t i = 0; i < num_to_destroy; ++i) 
-      allocator_.destroy(data_ + i);
+      data_[i].~T();
   } else {
     // skip gap or anything unused at the end
     for (size_t i = 0; i < h_; ++i)
-      allocator_.destroy(data_+ i);
+      data_[i].~T();
     
     for (size_t i = h_ + 1; i < h_ + r_ + 1; ++i)
-      allocator_.destroy(data_ + i);
+      data_[i].~T();
   }
 
   if (curr_items_alloc_ < prev_alloc) {
@@ -990,7 +990,7 @@ void var_opt_sketch<T, A>::grow_data_arrays() {
 
     for (uint32_t i = 0; i < prev_size; ++i) {
       new (&tmp_data[i]) T(std::move(data_[i]));
-      allocator_.destroy(data_ + i);
+      data_[i].~T();
       tmp_weights[i] = weights_[i];
     }
 
diff --git a/sampling/include/var_opt_union.hpp b/sampling/include/var_opt_union.hpp
index 1bc0b1b..3d00735 100644
--- a/sampling/include/var_opt_union.hpp
+++ b/sampling/include/var_opt_union.hpp
@@ -153,6 +153,8 @@ public:
 
 private:
   typedef typename std::allocator_traits<A>::template rebind_alloc<var_opt_sketch<T, A>> AllocSketch;
+  typedef typename std::allocator_traits<A>::template rebind_alloc<double> AllocDouble;
+  typedef typename std::allocator_traits<A>::template rebind_alloc<bool> AllocBool;
 
   static const uint8_t PREAMBLE_LONGS_EMPTY = 1;
   static const uint8_t PREAMBLE_LONGS_NON_EMPTY = 4;
@@ -170,10 +172,12 @@ private:
 
   uint32_t max_k_;
 
+  A allocator_;
+
   var_opt_sketch<T, A> gadget_;
 
   var_opt_union(uint64_t n, double outer_tau_numer, uint64_t outer_tau_denom,
-                uint32_t max_k, var_opt_sketch<T, A>&& gadget);
+                uint32_t max_k, var_opt_sketch<T, A>&& gadget, const A& allocator = A());
 
   /*
    IMPORTANT NOTE: the "gadget" in the union object appears to be a varopt sketch,
diff --git a/sampling/include/var_opt_union_impl.hpp b/sampling/include/var_opt_union_impl.hpp
index 1154061..b717f27 100644
--- a/sampling/include/var_opt_union_impl.hpp
+++ b/sampling/include/var_opt_union_impl.hpp
@@ -34,6 +34,7 @@ var_opt_union<T, A>::var_opt_union(uint32_t max_k, const A& allocator) :
   outer_tau_numer_(0.0),
   outer_tau_denom_(0),
   max_k_(max_k),
+  allocator_(allocator),
   gadget_(max_k, var_opt_sketch<T, A>::DEFAULT_RESIZE_FACTOR, true, allocator)
 {}
 
@@ -43,6 +44,7 @@ var_opt_union<T, A>::var_opt_union(const var_opt_union& other) :
   outer_tau_numer_(other.outer_tau_numer_),
   outer_tau_denom_(other.outer_tau_denom_),
   max_k_(other.max_k_),
+  allocator_(other.allocator_),
   gadget_(other.gadget_)
 {}
 
@@ -52,16 +54,18 @@ var_opt_union<T, A>::var_opt_union(var_opt_union&& other) noexcept :
   outer_tau_numer_(other.outer_tau_numer_),
   outer_tau_denom_(other.outer_tau_denom_),
   max_k_(other.max_k_),
+  allocator_(other.allocator_),
   gadget_(std::move(other.gadget_))
 {}
 
 template<typename T, typename A>
 var_opt_union<T, A>::var_opt_union(uint64_t n, double outer_tau_numer, uint64_t outer_tau_denom,
-                                    uint32_t max_k, var_opt_sketch<T, A>&& gadget) :
+                                    uint32_t max_k, var_opt_sketch<T, A>&& gadget, const A& allocator) :
   n_(n),
   outer_tau_numer_(outer_tau_numer),
   outer_tau_denom_(outer_tau_denom),
   max_k_(max_k),
+  allocator_(allocator),
   gadget_(gadget)
 {}
 
@@ -75,6 +79,7 @@ var_opt_union<T, A>& var_opt_union<T, A>::operator=(const var_opt_union& other)
   std::swap(outer_tau_numer_, union_copy.outer_tau_numer_);
   std::swap(outer_tau_denom_, union_copy.outer_tau_denom_);
   std::swap(max_k_, union_copy.max_k_);
+  std::swap(allocator_, other.allocator_);
   std::swap(gadget_, union_copy.gadget_);
   return *this;
 }
@@ -85,6 +90,7 @@ var_opt_union<T, A>& var_opt_union<T, A>::operator=(var_opt_union&& other) {
   std::swap(outer_tau_numer_, other.outer_tau_numer_);
   std::swap(outer_tau_denom_, other.outer_tau_denom_);
   std::swap(max_k_, other.max_k_);
+  std::swap(allocator_, other.allocator_);
   std::swap(gadget_, other.gadget_);
   return *this;
 }
@@ -162,7 +168,7 @@ var_opt_union<T, A> var_opt_union<T, A>::deserialize(std::istream& is, const Ser
   if (!is.good())
     throw std::runtime_error("error reading from std::istream"); 
 
-  return var_opt_union(items_seen, outer_tau_numer, outer_tau_denom, max_k, std::move(gadget));
+  return var_opt_union(items_seen, outer_tau_numer, outer_tau_denom, max_k, std::move(gadget), allocator);
 }
 
 template<typename T, typename A>
@@ -204,7 +210,7 @@ var_opt_union<T, A> var_opt_union<T, A>::deserialize(const void* bytes, size_t s
   const size_t gadget_size = size - (PREAMBLE_LONGS_NON_EMPTY << 3);
   var_opt_sketch<T, A> gadget = var_opt_sketch<T, A>::deserialize(ptr, gadget_size, sd, allocator);
 
-  return var_opt_union(items_seen, outer_tau_numer, outer_tau_denom, max_k, std::move(gadget));
+  return var_opt_union(items_seen, outer_tau_numer, outer_tau_denom, max_k, std::move(gadget), allocator);
 }
 
 template<typename T, typename A>
@@ -508,9 +514,8 @@ void var_opt_union<T, A>::mark_moving_gadget_coercer(var_opt_sketch<T, A>& sk) c
   uint32_t result_r = 0;
   size_t next_r_pos = result_k; // = (result_k+1)-1, to fill R region from back to front
 
-  typedef typename std::allocator_traits<A>::template rebind_alloc<double> AllocDouble;
-  double* wts = AllocDouble().allocate(result_k + 1);
-  T* data     = A().allocate(result_k + 1);
+  double* wts = AllocDouble(allocator_).allocate(result_k + 1);
+  T* data     = A(allocator_).allocate(result_k + 1);
     
   // insert R region items, ignoring weights
   // Currently (May 2017) this next block is unreachable; this coercer is used only in the
@@ -519,7 +524,7 @@ void var_opt_union<T, A>::mark_moving_gadget_coercer(var_opt_sketch<T, A>& sk) c
   // Addedndum (Jan 2020): Cleanup at end of method assumes R count is 0
   const size_t final_idx = gadget_.get_num_samples();
   for (size_t idx = gadget_.h_ + 1; idx <= final_idx; ++idx) {
-    A().construct(&data[next_r_pos], T(gadget_.data_[idx]));
+    new (&data[next_r_pos]) T(gadget_.data_[idx]);
     wts[next_r_pos]  = gadget_.weights_[idx];
     ++result_r;
     --next_r_pos;
@@ -530,13 +535,13 @@ void var_opt_union<T, A>::mark_moving_gadget_coercer(var_opt_sketch<T, A>& sk) c
   // insert H region items
   for (size_t idx = 0; idx < gadget_.h_; ++idx) {
     if (gadget_.marks_[idx]) {
-      A().construct(&data[next_r_pos], T(gadget_.data_[idx]));
+      new (&data[next_r_pos]) T(gadget_.data_[idx]);
       wts[next_r_pos] = -1.0;
       transferred_weight += gadget_.weights_[idx];
       ++result_r;
       --next_r_pos;
     } else {
-      A().construct(&data[result_h], T(gadget_.data_[idx]));
+      new (&data[result_h]) T(gadget_.data_[idx]);
       wts[result_h] = gadget_.weights_[idx];
       ++result_h;
     }
@@ -554,11 +559,10 @@ void var_opt_union<T, A>::mark_moving_gadget_coercer(var_opt_sketch<T, A>& sk) c
   wts[result_h] = -1.0;
 
   // clean up arrays in input sketch, replace with new values
-  typedef typename std::allocator_traits<A>::template rebind_alloc<bool> AllocBool;
-  AllocBool().deallocate(sk.marks_, sk.curr_items_alloc_);
-  AllocDouble().deallocate(sk.weights_, sk.curr_items_alloc_);
-  for (size_t i = 0; i < result_k; ++i) { A().destroy(sk.data_ + i); } // assumes everything in H region, no gap
-  A().deallocate(sk.data_, sk.curr_items_alloc_);
+  AllocBool(allocator_).deallocate(sk.marks_, sk.curr_items_alloc_);
+  AllocDouble(allocator_).deallocate(sk.weights_, sk.curr_items_alloc_);
+  for (size_t i = 0; i < result_k; ++i) { sk.data_[i].~T(); } // assumes everything in H region, no gap
+  A(allocator_).deallocate(sk.data_, sk.curr_items_alloc_);
 
   sk.data_ = data;
   sk.weights_ = wts;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org