From 6c6130817fc855f2e8b830c1b75bb2468488029f Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 28 Oct 2026 10:59:09 -0400 Subject: [PATCH 1/5] perf: kll_sketch allocates its items on first use The constructor allocated storage for k items up front, so every empty sketch held k * sizeof(T) bytes (1.6 KB for the default k with doubles). Code that creates many sketches that stay empty or small (one per group in a grouped aggregation) paid for all of it. Allocate the items on the first update or non-empty merge instead. The copy and converting constructors only allocate if the source has items; the destructor and the iterator already handle an unallocated buffer. Results and the serialized format are unchanged. Co-Authored-By: Claude Opus 5.5 --- kll/include/kll_sketch.hpp | 2 + kll/include/kll_sketch_impl.hpp | 22 ++++++++--- kll/test/kll_sketch_custom_type_test.cpp | 49 ++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index d672c419..6e22755d 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -572,6 +572,8 @@ class kll_sketch { // common update code inline void update_min_max(const T& item); + // allocates items_ (items_size_ items) unless it is allocated already + inline void allocate_items(); inline uint32_t internal_update(); // The following code is only valid in the special case of exactly reaching capacity while updating. diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index b12a39c8..de00d0a8 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -55,7 +55,8 @@ sorted_view_(nullptr) + std::to_string(kll_constants::MAX_K) + ": " + std::to_string(k)); } levels_[0] = levels_[1] = k; - items_ = allocator_.allocate(items_size_); + // items_ is allocated by the first update or merge: many sketches (e.g. per-group aggregation states) + // stay empty or are only ever merged into } template @@ -75,8 +76,10 @@ min_item_(other.min_item_), max_item_(other.max_item_), sorted_view_(nullptr) { - items_ = allocator_.allocate(items_size_); - for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); + if (other.items_ != nullptr) { + items_ = allocator_.allocate(items_size_); + for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); + } } template @@ -171,8 +174,10 @@ sorted_view_(nullptr) std::is_constructible::value, "Type converting constructor requires new type to be constructible from existing type" ); - items_ = allocator_.allocate(items_size_); - for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); + if (other.items_ != nullptr) { + items_ = allocator_.allocate(items_size_); + for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); + } check_sorting(); } @@ -197,8 +202,14 @@ void kll_sketch::update_min_max(const T& item) { } } +template +void kll_sketch::allocate_items() { + if (items_ == nullptr) items_ = allocator_.allocate(items_size_); +} + template uint32_t kll_sketch::internal_update() { + allocate_items(); if (levels_[0] == 0) { compress_while_updating(); } n_++; is_level_zero_sorted_ = false; @@ -212,6 +223,7 @@ void kll_sketch::merge(FwdSk&& other) { if (m_ != other.m_) { throw std::invalid_argument("incompatible M: " + std::to_string(m_) + " and " + std::to_string(other.m_)); } + allocate_items(); // the higher levels are merged into items_ even if other's level zero is empty if (is_empty()) { min_item_.emplace(conditional_forward(*other.min_item_)); max_item_.emplace(conditional_forward(*other.max_item_)); diff --git a/kll/test/kll_sketch_custom_type_test.cpp b/kll/test/kll_sketch_custom_type_test.cpp index f9d6f3cd..7195756f 100644 --- a/kll/test/kll_sketch_custom_type_test.cpp +++ b/kll/test/kll_sketch_custom_type_test.cpp @@ -153,4 +153,53 @@ TEST_CASE("kll sketch custom type", "[kll_sketch]") { REQUIRE(test_allocator_net_allocations == 0); } +TEST_CASE("kll sketch custom type: items are allocated on first use", "[kll_sketch]") { + test_allocator_total_bytes = 0; + test_allocator_net_allocations = 0; + { + kll_test_type_sketch sketch(200, test_type_less(), 0); + // only the levels: an empty sketch has no item storage + const long long empty_bytes = test_allocator_total_bytes; + REQUIRE(empty_bytes < static_cast(200 * sizeof(test_type))); + + kll_test_type_sketch copy(sketch); + kll_test_type_sketch converted(sketch, test_type_less(), alloc(0)); + REQUIRE(test_allocator_total_bytes == 3 * empty_bytes); + + kll_test_type_sketch target(200, test_type_less(), 0); + target.merge(sketch); // merging an empty sketch allocates nothing either + REQUIRE(test_allocator_total_bytes == 4 * empty_bytes); + REQUIRE(target.is_empty()); + + sketch.update(1); + REQUIRE(test_allocator_total_bytes == static_cast(4 * empty_bytes + 200 * sizeof(test_type))); + REQUIRE(sketch.get_n() == 1); + REQUIRE(sketch.get_min_item().get_value() == 1); + } + REQUIRE(test_allocator_total_bytes == 0); + REQUIRE(test_allocator_net_allocations == 0); +} + +TEST_CASE("kll sketch custom type: merge into a sketch without items", "[kll_sketch]") { + test_allocator_total_bytes = 0; + test_allocator_net_allocations = 0; + { + // estimation mode sizes, some of which leave level zero empty after a compaction + for (int n = 1; n < 2000; n += 37) { + kll_test_type_sketch source(8, test_type_less(), 0); + for (int i = 0; i < n; ++i) source.update(i); + kll_test_type_sketch target(8, test_type_less(), 0); + target.merge(source); + REQUIRE(target.get_n() == source.get_n()); + REQUIRE(target.get_min_item().get_value() == 0); + REQUIRE(target.get_max_item().get_value() == n - 1); + kll_test_type_sketch moved_target(8, test_type_less(), 0); + moved_target.merge(std::move(source)); + REQUIRE(moved_target.get_n() == target.get_n()); + } + } + REQUIRE(test_allocator_total_bytes == 0); + REQUIRE(test_allocator_net_allocations == 0); +} + } /* namespace datasketches */ From cc006f079caf5093feeacc8c6f395ae2f13d7b29 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Thu, 29 Oct 2026 12:18:28 -0400 Subject: [PATCH 2/5] fix: guard empty KLL sorted views before accessing items Check for an empty sketch in get_sorted_view before sorting level zero or forming item ranges. This shared entry point is the only caller of sort_level_zero, so no per-call-site null checks or eager allocation are needed. Exercise empty views after construction, copies, moves, assignments and deserialization, then verify that first updates still allocate and sort correctly. The new custom-type regression reproduces the null-pointer offset under UBSan before the fix. --- kll/include/kll_sketch.hpp | 2 + kll/include/kll_sketch_impl.hpp | 4 +- kll/test/kll_sketch_custom_type_test.cpp | 56 ++++++++++++++++++++++++ kll/test/kll_sketch_test.cpp | 4 ++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index 6e22755d..62c1d45b 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -521,6 +521,7 @@ class kll_sketch { /** * Gets the sorted view of this sketch + * An empty sketch returns an empty view without allocating item storage. * @return the sorted view of this sketch */ quantiles_sorted_view get_sorted_view() const; @@ -582,6 +583,7 @@ class kll_sketch { uint8_t find_level_to_compact() const; void add_empty_top_level_to_completely_full_sketch(); + // Requires a non-empty sketch; get_sorted_view() guards the call. void sort_level_zero(); template void merge_higher_levels(O&& other, uint64_t final_n); diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index de00d0a8..afac34af 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -779,8 +779,10 @@ void kll_sketch::check_sorting() const { template quantiles_sorted_view kll_sketch::get_sorted_view() const { - const_cast(this)->sort_level_zero(); // allow this side effect quantiles_sorted_view view(get_num_retained(), comparator_, allocator_); + // Guard both sorting and level ranges here: an empty sketch may have no item storage. + if (is_empty()) return view; + const_cast(this)->sort_level_zero(); // allow this side effect for (uint8_t level = 0; level < num_levels_; ++level) { const auto from = items_ + levels_[level]; const auto to = items_ + levels_[level + 1]; // exclusive diff --git a/kll/test/kll_sketch_custom_type_test.cpp b/kll/test/kll_sketch_custom_type_test.cpp index 7195756f..3161557c 100644 --- a/kll/test/kll_sketch_custom_type_test.cpp +++ b/kll/test/kll_sketch_custom_type_test.cpp @@ -202,4 +202,60 @@ TEST_CASE("kll sketch custom type: merge into a sketch without items", "[kll_ske REQUIRE(test_allocator_net_allocations == 0); } +TEST_CASE("kll sketch custom type: empty sorted view", "[kll_sketch]") { + test_allocator_total_bytes = 0; + test_allocator_net_allocations = 0; + { + kll_test_type_sketch empty(200, test_type_less(), 0); + kll_test_type_sketch copy(empty); + kll_test_type_sketch converted(empty, test_type_less(), alloc(0)); + kll_test_type_sketch moved(std::move(copy)); + + kll_test_type_sketch assigned(200, test_type_less(), 0); + assigned.update(99); + REQUIRE(assigned.get_quantile(0).get_value() == 99); // populate the cached view + assigned = empty; + + kll_test_type_sketch move_assigned(200, test_type_less(), 0); + move_assigned.update(99); + REQUIRE(move_assigned.get_quantile(0).get_value() == 99); + move_assigned = kll_test_type_sketch(empty); + + const auto bytes = empty.serialize(0, test_type_serde()); + auto from_bytes = kll_test_type_sketch::deserialize(bytes.data(), bytes.size(), + test_type_serde(), test_type_less(), alloc(0)); + std::stringstream stream; + empty.serialize(stream, test_type_serde()); + auto from_stream = kll_test_type_sketch::deserialize(stream, + test_type_serde(), test_type_less(), alloc(0)); + + for (auto* sketch: {&empty, &converted, &moved, &assigned, &move_assigned, &from_bytes, &from_stream}) { + REQUIRE(sketch->is_empty()); + REQUIRE(sketch->begin() == sketch->end()); + const auto bytes_before = test_allocator_total_bytes; + const auto allocations_before = test_allocator_net_allocations; + for (int i = 0; i < 2; ++i) { + const auto view = sketch->get_sorted_view(); + REQUIRE(view.size() == 0); + REQUIRE(view.begin() == view.end()); + REQUIRE_THROWS_AS(view.get_quantile(0.5), std::runtime_error); + REQUIRE(test_allocator_total_bytes == bytes_before); + REQUIRE(test_allocator_net_allocations == allocations_before); + } + + // Reading an empty view must not interfere with allocation or sorting on the first updates. + sketch->update(2); + sketch->update(1); + REQUIRE(sketch->get_n() == 2); + const auto view = sketch->get_sorted_view(); + REQUIRE(view.size() == 2); + REQUIRE(view.get_quantile(0).get_value() == 1); + REQUIRE(view.get_quantile(1).get_value() == 2); + REQUIRE(sketch->get_quantile(0).get_value() == 1); + } + } + REQUIRE(test_allocator_total_bytes == 0); + REQUIRE(test_allocator_net_allocations == 0); +} + } /* namespace datasketches */ diff --git a/kll/test/kll_sketch_test.cpp b/kll/test/kll_sketch_test.cpp index b8d0b031..b0b03273 100644 --- a/kll/test/kll_sketch_test.cpp +++ b/kll/test/kll_sketch_test.cpp @@ -75,6 +75,9 @@ TEST_CASE("kll sketch", "[kll_sketch]") { unused(pair); // to suppress "unused" warning FAIL("should be no iterations over an empty sketch"); } + const auto empty_bytes = test_allocator_total_bytes; + REQUIRE(sketch.get_sorted_view().size() == 0); + REQUIRE(test_allocator_total_bytes == empty_bytes); } SECTION("get bad quantile") { @@ -115,6 +118,7 @@ TEST_CASE("kll sketch", "[kll_sketch]") { kll_float_sketch sketch(200, std::less(), 0); sketch.update(std::numeric_limits::quiet_NaN()); REQUIRE(sketch.is_empty()); + REQUIRE(sketch.get_sorted_view().size() == 0); sketch.update(0); sketch.update(std::numeric_limits::quiet_NaN()); From 1fcf432fd30bd955b599c43e129f6e31028964cd Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Fri, 30 Oct 2026 10:48:40 -0400 Subject: [PATCH 3/5] refactor: handle unallocated KLL items in one place Review feedback: rather than each caller guarding against items_ being nullptr, keep that handling inside the helpers that own item storage: - allocate_items(): the only place items_ is allocated (update, merge) - copy_items(): the only place items are copied (copy and converting constructors), replacing two duplicated null checks - item_ptr(): the only place an item index becomes a pointer for readers; without item storage every range is empty, so sort_level_zero(), check_sorting() and get_sorted_view() need no guards of their own get_sorted_view() is back to its original form, without the is_empty() early return. UBSan flags the unguarded pointer arithmetic on an empty sketch ("applying non-zero offset to null pointer"), and passes with item_ptr(). Co-Authored-By: Claude Opus 5.5 --- kll/include/kll_sketch.hpp | 8 +++++-- kll/include/kll_sketch_impl.hpp | 37 +++++++++++++++++++-------------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index 62c1d45b..83e36c34 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -559,7 +559,7 @@ class kll_sketch { bool is_level_zero_sorted_; uint64_t n_; vector_u32 levels_; - T* items_; + T* items_; // nullptr until the sketch first receives items, see allocate_items() uint32_t items_size_; optional min_item_; optional max_item_; @@ -573,8 +573,13 @@ class kll_sketch { // common update code inline void update_min_max(const T& item); + // items_ is allocated lazily, and these are the only places that deal with it being nullptr: // allocates items_ (items_size_ items) unless it is allocated already inline void allocate_items(); + // copy-constructs the items of other, if other has any, into this newly constructed sketch + template void copy_items(const O& other); + // &items_[index], or nullptr without item storage, which makes every item range empty + inline T* item_ptr(uint32_t index) const; inline uint32_t internal_update(); // The following code is only valid in the special case of exactly reaching capacity while updating. @@ -583,7 +588,6 @@ class kll_sketch { uint8_t find_level_to_compact() const; void add_empty_top_level_to_completely_full_sketch(); - // Requires a non-empty sketch; get_sorted_view() guards the call. void sort_level_zero(); template void merge_higher_levels(O&& other, uint64_t final_n); diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index afac34af..49397722 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -76,10 +76,7 @@ min_item_(other.min_item_), max_item_(other.max_item_), sorted_view_(nullptr) { - if (other.items_ != nullptr) { - items_ = allocator_.allocate(items_size_); - for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); - } + copy_items(other); } template @@ -174,10 +171,7 @@ sorted_view_(nullptr) std::is_constructible::value, "Type converting constructor requires new type to be constructible from existing type" ); - if (other.items_ != nullptr) { - items_ = allocator_.allocate(items_size_); - for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); - } + copy_items(other); check_sorting(); } @@ -207,6 +201,19 @@ void kll_sketch::allocate_items() { if (items_ == nullptr) items_ = allocator_.allocate(items_size_); } +template +template +void kll_sketch::copy_items(const O& other) { + if (other.items_ == nullptr) return; + allocate_items(); + for (auto i = levels_[0]; i < levels_[num_levels_]; ++i) new (&items_[i]) T(other.items_[i]); +} + +template +T* kll_sketch::item_ptr(uint32_t index) const { + return items_ == nullptr ? nullptr : items_ + index; +} + template uint32_t kll_sketch::internal_update() { allocate_items(); @@ -760,7 +767,7 @@ void kll_sketch::add_empty_top_level_to_completely_full_sketch() { template void kll_sketch::sort_level_zero() { if (!is_level_zero_sorted_) { - std::sort(items_ + levels_[0], items_ + levels_[1], comparator_); + std::sort(item_ptr(levels_[0]), item_ptr(levels_[1]), comparator_); is_level_zero_sorted_ = true; } } @@ -769,8 +776,8 @@ template void kll_sketch::check_sorting() const { // not checking level 0 for (uint8_t level = 1; level < num_levels_; ++level) { - const auto from = items_ + levels_[level]; - const auto to = items_ + levels_[level + 1]; + const auto from = item_ptr(levels_[level]); + const auto to = item_ptr(levels_[level + 1]); if (!std::is_sorted(from, to, comparator_)) { throw std::logic_error("levels must be sorted"); } @@ -779,13 +786,11 @@ void kll_sketch::check_sorting() const { template quantiles_sorted_view kll_sketch::get_sorted_view() const { - quantiles_sorted_view view(get_num_retained(), comparator_, allocator_); - // Guard both sorting and level ranges here: an empty sketch may have no item storage. - if (is_empty()) return view; const_cast(this)->sort_level_zero(); // allow this side effect + quantiles_sorted_view view(get_num_retained(), comparator_, allocator_); for (uint8_t level = 0; level < num_levels_; ++level) { - const auto from = items_ + levels_[level]; - const auto to = items_ + levels_[level + 1]; // exclusive + const auto from = item_ptr(levels_[level]); + const auto to = item_ptr(levels_[level + 1]); // exclusive view.add(from, to, 1ULL << level); } view.convert_to_cummulative(); From a92aea34825b212e324836830824431f42821440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Stojko?= Date: Sat, 7 Nov 2026 20:53:58 +0000 Subject: [PATCH 4/5] perf: kll_sketch grows its items buffer by 8x up to k Builds on #558, which allocates the items buffer on the first update or merge. That allocation now has room for min(k, 200) items instead of k, and a full buffer smaller than k grows by RESIZE_FACTOR (X8) up to k. Compaction still starts only when the buffer holds k items, and while there is one level the serialized levels are offsets into a buffer of k items, so results and serialized bytes do not change. Co-authored-by: Isaac --- kll/include/kll_sketch.hpp | 10 +- kll/include/kll_sketch_impl.hpp | 39 +++++- kll/test/kll_sketch_test.cpp | 212 ++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 6 deletions(-) diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index 83e36c34..17e26215 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -39,6 +39,10 @@ namespace kll_constants { const uint16_t MIN_K = DEFAULT_M; /// max value of parameter K const uint16_t MAX_K = (1 << 16) - 1; + /// initial size of the items buffer if K is larger + const uint16_t INITIAL_ITEMS_SIZE = DEFAULT_K; + /// growth factor of the items buffer until it reaches K + const resize_factor RESIZE_FACTOR = resize_factor::X8; } /** @@ -183,7 +187,9 @@ class kll_sketch { using quantile_return_type = typename quantiles_sorted_view::quantile_return_type; /** - * Constructor + * Constructor. + * The first item allocates space for min(k, INITIAL_ITEMS_SIZE) items, + * which grows by RESIZE_FACTOR up to k items before the first compaction. * @param k affects the size of the sketch and its estimation error * @param comparator strict weak ordering function (see C++ named requirements: Compare) * @param allocator used by this sketch to allocate memory @@ -588,6 +594,8 @@ class kll_sketch { uint8_t find_level_to_compact() const; void add_empty_top_level_to_completely_full_sketch(); + void grow_items(); + uint32_t get_serialized_levels_offset() const; void sort_level_zero(); template void merge_higher_levels(O&& other, uint64_t final_n); diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index 49397722..1e518ffb 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -45,7 +45,7 @@ is_level_zero_sorted_(false), n_(0), levels_(2, 0, allocator), items_(nullptr), -items_size_(k_), +items_size_(std::min(k_, kll_constants::INITIAL_ITEMS_SIZE)), min_item_(), max_item_(), sorted_view_(nullptr) @@ -54,7 +54,7 @@ sorted_view_(nullptr) throw std::invalid_argument("K must be >= " + std::to_string(kll_constants::MIN_K) + " and <= " + std::to_string(kll_constants::MAX_K) + ": " + std::to_string(k)); } - levels_[0] = levels_[1] = k; + levels_[0] = levels_[1] = items_size_; // items_ is allocated by the first update or merge: many sketches (e.g. per-group aggregation states) // stay empty or are only ever merged into } @@ -217,7 +217,13 @@ T* kll_sketch::item_ptr(uint32_t index) const { template uint32_t kll_sketch::internal_update() { allocate_items(); - if (levels_[0] == 0) { compress_while_updating(); } + if (levels_[0] == 0) { + if (items_size_ < k_) { + grow_items(); + } else { + compress_while_updating(); + } + } n_++; is_level_zero_sorted_ = false; return --levels_[0]; @@ -408,7 +414,8 @@ void kll_sketch::serialize(std::ostream& os, const SerDe& sd) const { write(os, min_k_); write(os, num_levels_); write(os, unused); - write(os, levels_.data(), sizeof(levels_[0]) * num_levels_); + const uint32_t levels_offset = get_serialized_levels_offset(); + for (uint8_t i = 0; i < num_levels_; ++i) { write(os, levels_[i] + levels_offset); } sd.serialize(os, &*min_item_, 1); sd.serialize(os, &*max_item_, 1); } @@ -444,7 +451,8 @@ auto kll_sketch::serialize(unsigned header_size_bytes, const SerDe& sd) ptr += copy_to_mem(min_k_, ptr); ptr += copy_to_mem(num_levels_, ptr); ptr += sizeof(uint8_t); // unused - ptr += copy_to_mem(levels_.data(), ptr, sizeof(levels_[0]) * num_levels_); + const uint32_t levels_offset = get_serialized_levels_offset(); + for (uint8_t i = 0; i < num_levels_; ++i) { ptr += copy_to_mem(levels_[i] + levels_offset, ptr); } ptr += sd.serialize(ptr, end_ptr - ptr, &*min_item_, 1); ptr += sd.serialize(ptr, end_ptr - ptr, &*max_item_, 1); } @@ -764,6 +772,27 @@ void kll_sketch::add_empty_top_level_to_completely_full_sketch() { levels_[num_levels_] = new_total_cap; // initialize the new "extra" index at the top } +// Grows the items buffer of a one-level sketch when it is full and smaller than k. +// Compaction starts only when the buffer is full at k items, so growing does not change the result. +template +void kll_sketch::grow_items() { + const uint32_t new_size = std::min(items_size_ << static_cast(kll_constants::RESIZE_FACTOR), k_); + const uint32_t delta = new_size - items_size_; + T* new_items = allocator_.allocate(new_size); + kll_helper::move_construct(items_, levels_[0], levels_[1], new_items, levels_[0] + delta, true); + allocator_.deallocate(items_, items_size_); + items_ = new_items; + items_size_ = new_size; + levels_[0] += delta; + levels_[1] += delta; +} + +// Serialized levels are offsets into a buffer of k items while there is one level. +template +uint32_t kll_sketch::get_serialized_levels_offset() const { + return num_levels_ == 1 ? k_ - items_size_ : 0; +} + template void kll_sketch::sort_level_zero() { if (!is_level_zero_sorted_) { diff --git a/kll/test/kll_sketch_test.cpp b/kll/test/kll_sketch_test.cpp index b0b03273..fa1f5376 100644 --- a/kll/test/kll_sketch_test.cpp +++ b/kll/test/kll_sketch_test.cpp @@ -18,11 +18,13 @@ */ #include +#include #include #include #include #include #include +#include #include #include @@ -839,4 +841,214 @@ TEST_CASE("kll sketch", "[kll_sketch]") { REQUIRE(test_allocator_total_bytes == 0); } +// A deserialized sketch has a buffer of the full size. +template +static Sketch with_full_buffer(const Sketch& sketch) { + auto bytes = sketch.serialize(); + return Sketch::deserialize(bytes.data(), bytes.size(), serde(), + sketch.get_comparator(), sketch.get_allocator()); +} + +// Applies fn to both sketches with the same random bits, so that they compact the same way. +template +static void apply_to_both(Sketch& growing, Sketch& full, const Fn& fn) { + const auto random_bit = random_utils::random_bit; + fn(growing); + random_utils::random_bit = random_bit; + fn(full); +} + +template +static std::vector> items_and_weights(const Sketch& sketch) { + std::vector> result; + for (const auto pair: sketch) { result.emplace_back(pair.first, pair.second); } + return result; +} + +// Requires the same retained items and weights, the same query results and the same bytes. +template +static void require_identical(const Sketch& growing, const Sketch& full) { + REQUIRE(growing.get_n() == full.get_n()); + REQUIRE(growing.get_num_retained() == full.get_num_retained()); + REQUIRE(growing.is_estimation_mode() == full.is_estimation_mode()); + REQUIRE(growing.get_normalized_rank_error(false) == full.get_normalized_rank_error(false)); + REQUIRE(growing.get_normalized_rank_error(true) == full.get_normalized_rank_error(true)); + REQUIRE(items_and_weights(growing) == items_and_weights(full)); + if (!growing.is_empty()) { + REQUIRE(growing.get_min_item() == full.get_min_item()); + REQUIRE(growing.get_max_item() == full.get_max_item()); + std::vector split_points; + for (int i = 0; i <= 100; ++i) { + const double rank = i / 100.0; + for (const bool inclusive: {false, true}) { + const auto quantile = growing.get_quantile(rank, inclusive); + REQUIRE(quantile == full.get_quantile(rank, inclusive)); + REQUIRE(growing.get_rank(quantile, inclusive) == full.get_rank(quantile, inclusive)); + } + split_points.push_back(growing.get_quantile(rank)); + } + split_points.erase(std::unique(split_points.begin(), split_points.end()), split_points.end()); + for (const bool inclusive: {false, true}) { + REQUIRE(growing.get_PMF(split_points.data(), split_points.size(), inclusive) + == full.get_PMF(split_points.data(), split_points.size(), inclusive)); + REQUIRE(growing.get_CDF(split_points.data(), split_points.size(), inclusive) + == full.get_CDF(split_points.data(), split_points.size(), inclusive)); + } + } + REQUIRE(growing.serialize() == full.serialize()); + std::stringstream growing_stream, full_stream; + growing.serialize(growing_stream); + full.serialize(full_stream); + REQUIRE(growing_stream.str() == full_stream.str()); +} + +// Sizes of the items buffer as it grows by RESIZE_FACTOR up to k. +static std::vector buffer_sizes(uint16_t k) { + std::vector sizes = {std::min(k, kll_constants::INITIAL_ITEMS_SIZE)}; + while (sizes.back() < k) { + sizes.push_back(std::min(sizes.back() << static_cast(kll_constants::RESIZE_FACTOR), k)); + } + return sizes; +} + +// Updates a sketch and a copy of it with a full buffer the same way, and requires the same results. +template +static void require_same_as_full_buffer(Sketch& growing, uint32_t num_items) { + auto full = with_full_buffer(growing); + random_utils::random_bit.seed(4); + for (uint32_t i = 0; i < num_items; ++i) { + const auto item = static_cast(i % 7777); + apply_to_both(growing, full, [&item](Sketch& sketch) { sketch.update(item); }); + if (i == 1000 || i == 20000) { require_identical(growing, full); } + } + require_identical(growing, full); +} + +TEST_CASE("kll sketch: lazy growth of the items buffer", "[kll_sketch]") { + test_allocator_total_bytes = 0; + const long long levels_bytes = 2 * sizeof(uint32_t); + + SECTION("k not above the initial size allocates k items with the first item") { + kll_float_sketch sketch(100, std::less(), 0); + REQUIRE(test_allocator_total_bytes == levels_bytes); + for (int i = 0; i < 100; ++i) { + sketch.update(static_cast(i)); + REQUIRE(test_allocator_total_bytes == static_cast(levels_bytes + 100 * sizeof(float))); + } + } + + SECTION("buffer grows by RESIZE_FACTOR up to k") { + REQUIRE(buffer_sizes(kll_constants::MAX_K) == std::vector{200, 1600, 12800, 65535}); + kll_float_sketch sketch(kll_constants::MAX_K, std::less(), 0); + REQUIRE(test_allocator_total_bytes == levels_bytes); + uint32_t n = 0; + for (const uint32_t size: buffer_sizes(kll_constants::MAX_K)) { + while (n < size) { + sketch.update(static_cast(n++)); + // the buffer grows when an item does not fit + REQUIRE(test_allocator_total_bytes == static_cast(levels_bytes + size * sizeof(float))); + } + } + REQUIRE_FALSE(sketch.is_estimation_mode()); + sketch.update(0); + REQUIRE(sketch.is_estimation_mode()); + } + + SECTION("first merge allocates the initial size") { + kll_float_sketch other(kll_constants::MAX_K, std::less(), 0); + for (int i = 0; i < 50; ++i) { other.update(static_cast(i)); } + const long long other_bytes = test_allocator_total_bytes; + kll_float_sketch sketch(kll_constants::MAX_K, std::less(), 0); + sketch.merge(other); + REQUIRE(test_allocator_total_bytes - other_bytes + == static_cast(levels_bytes + kll_constants::INITIAL_ITEMS_SIZE * sizeof(float))); + REQUIRE(sketch.get_n() == 50); + } + + SECTION("same as a full buffer through compactions") { + // the buffer grows 1, 2 and 3 times before it reaches these k + const uint16_t ks[] = {201, 5000, 20000}; + for (const uint16_t k: ks) { + random_utils::random_bit.seed(k); + kll_float_sketch growing(k, std::less(), 0); + growing.update(0); + const long long growing_bytes = test_allocator_total_bytes; + kll_float_sketch full = with_full_buffer(growing); + REQUIRE(test_allocator_total_bytes - growing_bytes == static_cast(levels_bytes + k * sizeof(float))); + const auto sizes = buffer_sizes(k); + for (uint32_t n = 2; n <= 10u * k; ++n) { + const float item = static_cast((n * 7919) % 10007) / 3; + apply_to_both(growing, full, [item](kll_float_sketch& sketch) { sketch.update(item); }); + const bool at_growth = std::find(sizes.begin(), sizes.end(), n) != sizes.end() + || std::find(sizes.begin(), sizes.end(), n - 1) != sizes.end(); + if (at_growth || n % 997 == 0 || n % k <= 1) { require_identical(growing, full); } + } + REQUIRE(growing.is_estimation_mode()); + require_identical(growing, full); + } + } + + SECTION("same as a full buffer when merging") { + random_utils::random_bit.seed(3); + // exact mode sketches first, so that merging grows the buffer before compactions start + const std::vector> k_and_n = { + {30000, 1000}, {30000, 3000}, {20000, 15000}, {2000, 10000}, {8, 100}, {20000, 60000}}; + std::vector others; + for (const auto& shape: k_and_n) { + others.emplace_back(shape.first, std::less(), 0); + for (uint32_t i = 0; i < shape.second; ++i) { others.back().update(static_cast((i * 31) % 4999)); } + } + for (const bool reversed: {false, true}) { + kll_float_sketch growing(20000, std::less(), 0); + growing.update(0); + kll_float_sketch full = with_full_buffer(growing); + for (size_t i = 0; i < others.size(); ++i) { + const auto& other = others[reversed ? others.size() - 1 - i : i]; + apply_to_both(growing, full, [&other](kll_float_sketch& sketch) { sketch.merge(other); }); + require_identical(growing, full); + } + for (int i = 0; i < 50000; ++i) { + apply_to_both(growing, full, [i](kll_float_sketch& sketch) { sketch.update(static_cast(i)); }); + } + require_identical(growing, full); + } + } + + SECTION("copies, moves and converted sketches keep growing the same") { + kll_float_sketch sketch(kll_constants::MAX_K, std::less(), 0); + for (int i = 0; i < 500; ++i) { sketch.update(static_cast(i)); } + kll_float_sketch copied(sketch); + kll_float_sketch assigned(8, std::less(), 0); + assigned = sketch; + kll_float_sketch to_move(sketch); + kll_float_sketch moved(std::move(to_move)); + kll_float_sketch move_assigned(8, std::less(), 0); + kll_float_sketch to_move_assign(sketch); + move_assigned = std::move(to_move_assign); + for (auto* growing: {&sketch, &copied, &assigned, &moved, &move_assigned}) { + require_same_as_full_buffer(*growing, 200000); + } + kll_float_sketch source(kll_constants::MAX_K, std::less(), 0); + for (int i = 0; i < 500; ++i) { source.update(static_cast(i)); } + kll_sketch, test_allocator> converted(source, std::less(), 0); + require_same_as_full_buffer(converted, 200000); + } + + SECTION("same as a full buffer with strings") { + random_utils::random_bit.seed(5); + kll_string_sketch growing(5000, std::less(), 0); + growing.update("0"); + kll_string_sketch full = with_full_buffer(growing); + for (int i = 1; i < 30000; ++i) { + const std::string item = std::to_string((i * 7919) % 10007); + apply_to_both(growing, full, [&item](kll_string_sketch& sketch) { sketch.update(item); }); + if (i == 199 || i == 200 || i == 1599 || i == 1600 || i == 4999 || i == 10000) { require_identical(growing, full); } + } + require_identical(growing, full); + } + + // cleanup + REQUIRE(test_allocator_total_bytes == 0); +} + } /* namespace datasketches */ From f3237e89a1281c6df962d443d3a02f6cbf1edab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Stojko?= Date: Sat, 7 Nov 2026 22:56:24 +0000 Subject: [PATCH 5/5] KLL: add benchmarks for building, merging and serializing Co-authored-by: Isaac --- benchmarks/CMakeLists.txt | 17 +++ benchmarks/benchmark_kll_sketch.cpp | 163 ++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 benchmarks/benchmark_kll_sketch.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index f543dfce..342e4478 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -50,3 +50,20 @@ target_sources(count_min_sketch_benchmark benchmark_count_min_sketch.cpp benchmark_count_min_sketch_serialization.cpp ) + +add_executable(kll_sketch_benchmark) + +target_link_libraries(kll_sketch_benchmark + PRIVATE + kll + benchmark::benchmark_main +) + +set_target_properties(kll_sketch_benchmark PROPERTIES + CXX_STANDARD_REQUIRED YES +) + +target_sources(kll_sketch_benchmark + PRIVATE + benchmark_kll_sketch.cpp +) diff --git a/benchmarks/benchmark_kll_sketch.cpp b/benchmarks/benchmark_kll_sketch.cpp new file mode 100644 index 00000000..fea5b979 --- /dev/null +++ b/benchmarks/benchmark_kll_sketch.cpp @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include + +#include +#include +#include + +namespace +{ + +using Sketch = datasketches::kll_sketch; + +std::vector makeItems(size_t size) +{ + std::vector items; + items.reserve(size); + uint64_t state = 1; + for (size_t i = 0; i < size; ++i) + { + state = state * 6364136223846793005ULL + 1442695040888963407ULL; + items.push_back(static_cast(state >> 11)); + } + return items; +} + +// A new sketch with k = range(0) gets range(1) items. +void BM_KllBuild(benchmark::State & state) +{ + const auto k = static_cast(state.range(0)); + const auto items = makeItems(static_cast(state.range(1))); + for (auto _ : state) + { + Sketch sketch(k); + for (const double item : items) + sketch.update(item); + + benchmark::DoNotOptimize(sketch.get_n()); + benchmark::ClobberMemory(); + } + + state.SetItemsProcessed(state.iterations() * static_cast(items.size())); +} + +// Many sketches with k = range(0) are alive at once, each with range(1) items, like the groups of +// a GROUP BY. +void BM_KllBuildManySketches(benchmark::State & state) +{ + constexpr size_t NUM_SKETCHES = 1000; + const auto k = static_cast(state.range(0)); + const auto items = makeItems(static_cast(state.range(1))); + for (auto _ : state) + { + std::vector sketches(NUM_SKETCHES, Sketch(k)); + for (auto & sketch : sketches) + for (const double item : items) + sketch.update(item); + + benchmark::DoNotOptimize(sketches.data()); + benchmark::ClobberMemory(); + } + + state.SetItemsProcessed(state.iterations() * static_cast(NUM_SKETCHES * items.size())); +} + +// A new sketch with k = range(0) merges 100 sketches of range(1) items each. +void BM_KllMergeInto(benchmark::State & state) +{ + constexpr size_t NUM_INPUTS = 100; + const auto k = static_cast(state.range(0)); + const auto items = makeItems(static_cast(state.range(1))); + std::vector inputs(NUM_INPUTS, Sketch(k)); + for (auto & input : inputs) + for (const double item : items) + input.update(item); + + for (auto _ : state) + { + Sketch sketch(k); + for (const auto & input : inputs) + sketch.merge(input); + + benchmark::DoNotOptimize(sketch.get_n()); + benchmark::ClobberMemory(); + } + + state.SetItemsProcessed(state.iterations() * static_cast(NUM_INPUTS * items.size())); +} + +// Serializes a sketch with k = range(0) and range(1) items. +void BM_KllSerialize(benchmark::State & state) +{ + Sketch sketch(static_cast(state.range(0))); + for (const double item : makeItems(static_cast(state.range(1)))) + sketch.update(item); + + for (auto _ : state) + { + auto bytes = sketch.serialize(); + benchmark::DoNotOptimize(bytes.data()); + benchmark::ClobberMemory(); + } +} + +void buildArgs(benchmark::internal::Benchmark * b) +{ + for (const int64_t k : {200, 1000, 5000, 20000, 65535}) + { + // n = k fills the buffer up to k with no compaction, so it includes every growth step + for (const int64_t n : {int64_t{1}, int64_t{10}, int64_t{100}, int64_t{1000}, k, int64_t{10000}, + int64_t{100000}, int64_t{1000000}}) + { + if (n <= 100 * k) + b->Args({k, n}); + } + } +} + +void manySketchesArgs(benchmark::internal::Benchmark * b) +{ + for (const int64_t k : {200, 1000, 5000, 20000, 65535}) + for (const int64_t n : {1, 10, 100, 1000}) + b->Args({k, n}); +} + +void mergeArgs(benchmark::internal::Benchmark * b) +{ + for (const int64_t k : {200, 5000, 65535}) + for (const int64_t n : {10, 100, 1000}) + b->Args({k, n}); +} + +void serializeArgs(benchmark::internal::Benchmark * b) +{ + for (const int64_t k : {200, 5000, 65535}) + for (const int64_t n : {10, 1000, 100000}) + b->Args({k, n}); +} + +} + +BENCHMARK(BM_KllBuild)->Apply(buildArgs); +BENCHMARK(BM_KllBuildManySketches)->Apply(manySketchesArgs)->Unit(benchmark::kMicrosecond); +BENCHMARK(BM_KllMergeInto)->Apply(mergeArgs)->Unit(benchmark::kMicrosecond); +BENCHMARK(BM_KllSerialize)->Apply(serializeArgs);