refactor: remove update sketch

Author: proost

tuple/include/array_of_strings_sketch.hpp

@@ -69,6 +69,12 @@ struct default_array_of_strings_serde {
   static void check_utf8(const std::string& value);
 };
 
+/**
+ * Hashes an array of strings using ArrayOfStrings-compatible hashing.
+ */
+template<typename Allocator = std::allocator<std::string>>
+uint64_t hash_array_of_strings_key(const array<std::string, Allocator>& key);
+
 /**
  * Extended class of compact_tuple_sketch for array of strings
  * Requirements: all strings must be valid UTF-8 and array size must be <= 127.
@@ -125,63 +131,26 @@ class compact_array_of_strings_tuple_sketch:
 };
 
 /**
- * Extended class of update_tuple_sketch for array of strings
+ * Convenience alias for update_tuple_sketch for array of strings
  */
-template<template<typename> class Policy = default_array_of_strings_update_policy,
-         typename Allocator = std::allocator<std::string>>
-class update_array_of_strings_tuple_sketch:
-  public update_tuple_sketch<
-    array<std::string, Allocator>,
-    array<std::string, Allocator>,
-    Policy<Allocator>,
-    typename std::allocator_traits<Allocator>::template rebind_alloc<array<std::string, Allocator>>
-  > {
-public:
-  using array_of_strings = array<std::string, Allocator>;
-  using summary_allocator = typename std::allocator_traits<Allocator>::template rebind_alloc<array_of_strings>;
-  using policy_type = Policy<Allocator>;
-  using Base = update_tuple_sketch<
-    array_of_strings,
-    array_of_strings,
-    policy_type,
-    summary_allocator
-  >;
-  using resize_factor = typename Base::resize_factor;
-  class builder;
-  using Base::update;
-
-  /**
-   * Updates the sketch with string array for both key and value.
-   * @param key the given string array key
-   * @param value the given string array value
-   */
-  void update(const array_of_strings& key, const array_of_strings& value);
+template<typename Allocator = std::allocator<std::string>,
+         typename Policy = default_array_of_strings_update_policy<Allocator>>
+using update_array_of_strings_tuple_sketch = update_tuple_sketch<
+  array<std::string, Allocator>,
+  array<std::string, Allocator>,
+  Policy,
+  typename std::allocator_traits<Allocator>::template rebind_alloc<array<std::string, Allocator>>
+>;
 
-  /**
-   * Converts this sketch to a compact sketch (ordered or unordered).
-   * @param ordered optional flag to specify if an ordered sketch should be produced
-   * @return compact array of strings sketch
-   */
-  compact_array_of_strings_tuple_sketch<Allocator> compact(bool ordered = true) const;
-
-private:
-  update_array_of_strings_tuple_sketch(uint8_t lg_cur_size, uint8_t lg_nom_size, resize_factor rf, float p, uint64_t theta,
-      uint64_t seed, const policy_type& policy, const summary_allocator& allocator);
-
-  // Matches Java Util.PRIME for ArrayOfStrings key hashing.
-  static constexpr uint64_t STRING_ARR_HASH_SEED = 0x7A3CCA71ULL;
-
-  static uint64_t hash_key(const array_of_strings& key);
-};
-
-template<template<typename> class Policy, typename Allocator>
-class update_array_of_strings_tuple_sketch<Policy, Allocator>::builder:
-  public tuple_base_builder<builder, policy_type, summary_allocator> {
-public:
-  builder(const policy_type& policy = policy_type(), const summary_allocator& allocator = summary_allocator());
-
-  update_array_of_strings_tuple_sketch build() const;
-};
+/**
+ * Converts an array of strings tuple sketch to a compact sketch (ordered or unordered).
+ * @param sketch input sketch
+ * @param ordered optional flag to specify if an ordered sketch should be produced
+ * @return compact array of strings sketch
+ */
+template<typename Allocator = std::allocator<std::string>, typename Policy = default_array_of_strings_update_policy<Allocator>>
+compact_array_of_strings_tuple_sketch<Allocator> compact_array_of_strings_sketch(
+  const update_array_of_strings_tuple_sketch<Allocator, Policy>& sketch, bool ordered = true);
 
 } /* namespace datasketches */
 

tuple/include/array_of_strings_sketch_impl.hpp

@@ -58,23 +58,10 @@ void default_array_of_strings_update_policy<Allocator>::update(
   for (size_t i = 0; i < length; ++i) array[i] = (*input)[i];
 }
 
-template<template<typename> class Policy, typename Allocator>
-update_array_of_strings_tuple_sketch<Policy, Allocator>::update_array_of_strings_tuple_sketch(
-  uint8_t lg_cur_size, uint8_t lg_nom_size, resize_factor rf, float p, uint64_t theta,
-  uint64_t seed, const policy_type& policy, const summary_allocator& allocator
-):
-Base(lg_cur_size, lg_nom_size, rf, p, theta, seed, policy, allocator) {}
-
-template<template<typename> class Policy, typename Allocator>
-void update_array_of_strings_tuple_sketch<Policy, Allocator>::update(
-  const array_of_strings& key, const array_of_strings& value
-) {
-  const uint64_t hash = hash_key(key);
-  Base::update(hash, value);
-}
-
-template<template<typename> class Policy, typename Allocator>
-uint64_t update_array_of_strings_tuple_sketch<Policy, Allocator>::hash_key(const array_of_strings& key) {
+template<typename Allocator>
+uint64_t hash_array_of_strings_key(const array<std::string, Allocator>& key) {
+  // Matches Java Util.PRIME for ArrayOfStrings key hashing.
+  static constexpr uint64_t STRING_ARR_HASH_SEED = 0x7A3CCA71ULL;
   XXHash64 hasher(STRING_ARR_HASH_SEED);
   const auto size = static_cast<size_t>(key.size());
   for (size_t i = 0; i < size; ++i) {
@@ -85,31 +72,11 @@ uint64_t update_array_of_strings_tuple_sketch<Policy, Allocator>::hash_key(const
   return hasher.hash();
 }
 
-template<template<typename> class Policy, typename Allocator>
-compact_array_of_strings_tuple_sketch<Allocator> update_array_of_strings_tuple_sketch<Policy, Allocator>::compact(bool ordered) const {
-  return compact_array_of_strings_tuple_sketch<Allocator>(*this, ordered);
-}
-
-// builder
-
-template<template<typename> class Policy, typename Allocator>
-update_array_of_strings_tuple_sketch<Policy, Allocator>::builder::builder(
-  const policy_type& policy, const summary_allocator& allocator
-):
-tuple_base_builder<builder, policy_type, summary_allocator>(policy, allocator) {}
-
-template<template<typename> class Policy, typename Allocator>
-auto update_array_of_strings_tuple_sketch<Policy, Allocator>::builder::build() const -> update_array_of_strings_tuple_sketch {
-  return update_array_of_strings_tuple_sketch(
-    this->starting_lg_size(),
-    this->lg_k_,
-    this->rf_,
-    this->p_,
-    this->starting_theta(),
-    this->seed_,
-    this->policy_,
-    this->allocator_
-  );
+template<typename Allocator, typename Policy>
+compact_array_of_strings_tuple_sketch<Allocator> compact_array_of_strings_sketch(
+  const update_array_of_strings_tuple_sketch<Allocator, Policy>& sketch, bool ordered
+) {
+  return compact_array_of_strings_tuple_sketch<Allocator>(sketch, ordered);
 }
 
 template<typename Allocator>
@@ -183,7 +150,7 @@ void default_array_of_strings_serde<Allocator>::deserialize(
       if (!is) throw std::runtime_error("array_of_strings stream read failed");
       std::string value(length, '\0');
       if (length != 0) {
-        is.read(value.data(), length);
+        is.read(&value[0], length);
         if (!is) throw std::runtime_error("array_of_strings stream read failed");
       }
       check_utf8(value);
@@ -241,7 +208,7 @@ size_t default_array_of_strings_serde<Allocator>::deserialize(
       bytes_read += copy_from_mem(ptr8 + bytes_read, length);
       std::string value(length, '\0');
       if (length != 0) {
-        bytes_read += copy_from_mem(ptr8 + bytes_read, value.data(), length);
+        bytes_read += copy_from_mem(ptr8 + bytes_read, &value[0], length);
       }
       check_utf8(value);
       array[j] = std::move(value);

tuple/test/aos_sketch_serialize_for_java.cpp

@@ -26,7 +26,7 @@
 namespace datasketches {
 
 using aos_sketch = update_array_of_strings_tuple_sketch<>;
-using array_of_strings = aos_sketch::array_of_strings;
+using array_of_strings = array<std::string>;
 
 static array_of_strings make_array(std::initializer_list<std::string> items) {
   array_of_strings array(static_cast<uint8_t>(items.size()), "");
@@ -47,12 +47,12 @@ TEST_CASE("aos sketch generate one value", "[serialize_for_java]") {
       key[0] = std::to_string(i);
       array_of_strings value(1, "");
       value[0] = "value" + std::to_string(i);
-      sketch.update(key, value);
+      sketch.update(hash_array_of_strings_key(key), value);
     }
     REQUIRE(sketch.is_empty() == (n == 0));
     REQUIRE(sketch.get_estimate() == Approx(n).margin(n * 0.03));
     std::ofstream os("aos_1_n" + std::to_string(n) + "_cpp.sk", std::ios::binary);
-    sketch.compact().serialize(os, default_array_of_strings_serde<>());
+    compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
   }
 }
 
@@ -67,12 +67,12 @@ TEST_CASE("aos sketch generate three values", "[serialize_for_java]") {
       value[0] = "a" + std::to_string(i);
       value[1] = "b" + std::to_string(i);
       value[2] = "c" + std::to_string(i);
-      sketch.update(key, value);
+      sketch.update(hash_array_of_strings_key(key), value);
     }
     REQUIRE(sketch.is_empty() == (n == 0));
     REQUIRE(sketch.get_estimate() == Approx(n).margin(n * 0.03));
     std::ofstream os("aos_3_n" + std::to_string(n) + "_cpp.sk", std::ios::binary);
-    sketch.compact().serialize(os, default_array_of_strings_serde<>());
+    compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
   }
 }
 
@@ -86,11 +86,11 @@ TEST_CASE("aos sketch generate non-empty no entries", "[serialize_for_java]") {
   key[0] = "key1";
   array_of_strings value(1, "");
   value[0] = "value1";
-  sketch.update(key, value);
+  sketch.update(hash_array_of_strings_key(key), value);
   REQUIRE_FALSE(sketch.is_empty());
   REQUIRE(sketch.get_num_retained() == 0);
   std::ofstream os("aos_1_non_empty_no_entries_cpp.sk", std::ios::binary);
-  sketch.compact().serialize(os, default_array_of_strings_serde<>());
+  compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
 }
 
 TEST_CASE("aos sketch generate multi key strings", "[serialize_for_java]") {
@@ -103,53 +103,44 @@ TEST_CASE("aos sketch generate multi key strings", "[serialize_for_java]") {
       key[1] = "subkey" + std::to_string(i % 10);
       array_of_strings value(1, "");
       value[0] = "value" + std::to_string(i);
-      sketch.update(key, value);
+      sketch.update(hash_array_of_strings_key(key), value);
     }
     REQUIRE(sketch.is_empty() == (n == 0));
     REQUIRE(sketch.get_estimate() == Approx(n).margin(n * 0.03));
     std::ofstream os("aos_multikey_n" + std::to_string(n) + "_cpp.sk", std::ios::binary);
-    sketch.compact().serialize(os, default_array_of_strings_serde<>());
+    compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
   }
 }
 
 TEST_CASE("aos sketch generate unicode strings", "[serialize_for_java]") {
   auto sketch = aos_sketch::builder().build();
   sketch.update(
-    make_array({u8"키", u8"열쇠"}),
+    hash_array_of_strings_key(make_array({u8"키", u8"열쇠"})),
     make_array({u8"밸류", u8"값"})
   );
   sketch.update(
-    make_array({u8"🔑", u8"🗝️"}),
+    hash_array_of_strings_key(make_array({u8"🔑", u8"🗝️"})),
     make_array({u8"📦", u8"🎁"})
   );
   sketch.update(
-    make_array({u8"ключ1", u8"ключ2"}),
+    hash_array_of_strings_key(make_array({u8"ключ1", u8"ключ2"})),
     make_array({u8"ценить1", u8"ценить2"})
   );
   REQUIRE_FALSE(sketch.is_empty());
   REQUIRE(sketch.get_num_retained() == 3);
   std::ofstream os("aos_unicode_cpp.sk", std::ios::binary);
-  sketch.compact().serialize(os, default_array_of_strings_serde<>());
+  compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
 }
 
 TEST_CASE("aos sketch generate empty strings", "[serialize_for_java]") {
   auto sketch = aos_sketch::builder().build();
-  sketch.update(
-    make_array({""}),
-    make_array({"empty_key_value"})
-  );
-  sketch.update(
-    make_array({"empty_value_key"}),
-    make_array({""})
-  );
-  sketch.update(
-    make_array({"", ""}),
-    make_array({"", ""})
-  );
+  sketch.update(hash_array_of_strings_key(make_array({""})), make_array({"empty_key_value"}));
+  sketch.update(hash_array_of_strings_key(make_array({"empty_value_key"})), make_array({""}));
+  sketch.update(hash_array_of_strings_key(make_array({"", ""})), make_array({"", ""}));
   REQUIRE_FALSE(sketch.is_empty());
   REQUIRE(sketch.get_num_retained() == 3);
   std::ofstream os("aos_empty_strings_cpp.sk", std::ios::binary);
-  sketch.compact().serialize(os, default_array_of_strings_serde<>());
+  compact_array_of_strings_sketch(sketch).serialize(os, default_array_of_strings_serde<>());
 }
 
 } /* namespace datasketches */