From 30093b2d251abc89cfd40ecfc730f757c0f942fe Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 31 Oct 2026 11:45:52 +0800 Subject: [PATCH 1/9] perf(hll): copy dense HLL8 inputs without rebuilding estimates Adopting an HLL8 sketch into an empty union previously allocated a zeroed register array, merged the source registers, and rebuilt the zero count and KxQ caches. Recreating a union for each small partial therefore scanned the entire accumulated dense sketch even when no downsampling was needed. Clone the source array and its estimator when its precision fits within the union's maximum. This also covers a smaller source precision and the reverse-merge path that adopts a dense input before replaying coupons. Representation conversions and actual downsampling retain their existing paths. Add regression coverage for subsequent updates after copying both HIP and composite estimates. Add benchmarks for empty-union adoption, repeated union construction, persistent small-partial merges, and dense-partial merges at lg_k 12 and 18. Validation: all 46 HLL integration tests pass. Release benchmarks confirm that dense-input adoption and repeated union construction avoid the previous estimator-rebuild cost; persistent small-partial merges remain on their existing path. --- CHANGELOG.md | 1 + benchmarks/hll/mod.rs | 1 + benchmarks/hll/union.rs | 83 +++++++++++++++++++++++ datasketches/src/hll/union.rs | 12 ++-- tests-integration/tests/hll_test/union.rs | 28 ++++++++ 5 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 benchmarks/hll/union.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cbc6000b..8ebddcf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All significant changes to this project will be documented in this file. ### Improvements +* `HllUnion` is faster when adopting an HLL8 input without downsampling, including repeated unions of a dense result with small partial sketches. * `KllSketch::rank` is faster for repeated queries on compacted sketches. * Repeated scalar `TDigest::rank` queries are faster, including small digests and data with repeated values. * `TDigest` and `TDigestMut` CDF and PMF batches are faster for many split points; batches with sparse split points keep the original binary-search behavior. diff --git a/benchmarks/hll/mod.rs b/benchmarks/hll/mod.rs index 1dea33be..e56cecc9 100644 --- a/benchmarks/hll/mod.rs +++ b/benchmarks/hll/mod.rs @@ -15,4 +15,5 @@ // specific language governing permissions and limitations // under the License. +mod union; mod update; diff --git a/benchmarks/hll/union.rs b/benchmarks/hll/union.rs new file mode 100644 index 00000000..59e5639c --- /dev/null +++ b/benchmarks/hll/union.rs @@ -0,0 +1,83 @@ +// 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. + +use datasketches::hll::HllSketch; +use datasketches::hll::HllType; +use datasketches::hll::HllUnion; +use divan::Bencher; +use divan::black_box; + +fn partials(lg_k: u8, count: usize, rows: usize) -> Vec { + (0..count) + .map(|partial| { + let mut sketch = HllSketch::new(lg_k, HllType::Hll8).unwrap(); + for value in partial * rows..(partial + 1) * rows { + sketch.update(value as u64); + } + sketch + }) + .collect() +} + +#[divan::bench(args = [12, 18])] +fn empty_from_dense(bencher: Bencher, lg_k: u8) { + let source = partials(lg_k, 1, 1 << lg_k).pop().unwrap(); + bencher.bench_local(|| { + let mut union = HllUnion::new(lg_k).unwrap(); + union.update(black_box(&source)); + black_box(union) + }); +} + +#[divan::bench(args = [12, 18])] +fn recreated_small_partials(bencher: Bencher, lg_k: u8) { + let partials = partials(lg_k, 400, 200); + bencher.bench_local(|| { + let mut sketch = HllSketch::new(lg_k, HllType::Hll8).unwrap(); + for partial in black_box(&partials) { + let mut union = HllUnion::new(lg_k).unwrap(); + union.update(&sketch); + union.update(partial); + sketch = union.to_sketch(HllType::Hll8); + } + black_box(sketch) + }); +} + +#[divan::bench(args = [12, 18])] +fn persistent_small_partials(bencher: Bencher, lg_k: u8) { + let partials = partials(lg_k, 400, 200); + bencher.bench_local(|| { + let mut union = HllUnion::new(lg_k).unwrap(); + for partial in black_box(&partials) { + union.update(partial); + } + black_box(union.to_sketch(HllType::Hll8)) + }); +} + +#[divan::bench(args = [12, 18])] +fn dense_partials(bencher: Bencher, lg_k: u8) { + let partials = partials(lg_k, 16, (1 << lg_k) / 4); + bencher.bench_local(|| { + let mut union = HllUnion::new(lg_k).unwrap(); + for partial in black_box(&partials) { + union.update(partial); + } + black_box(union.to_sketch(HllType::Hll8)) + }); +} diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index b245fa71..436198ee 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -586,17 +586,17 @@ fn copy_or_downsample(src_mode: &Mode, src_lg_k: u8, tgt_lg_k: u8) -> Array8 { let estimate_state = get_array_estimate_state(src_mode); let mut result = if src_lg_k <= tgt_lg_k { - let mut result = Array8::new(src_lg_k); - match src_mode { - Mode::Array8(src) => { - result.merge_array_same_lgk(src.values()); - } + Mode::Array8(src) => src.clone(), Mode::Array6(src) => { + let mut result = Array8::new(src_lg_k); copy_array46_via_coupons(&mut result, src.num_registers(), |slot| src.get(slot)); + result } Mode::Array4(src) => { + let mut result = Array8::new(src_lg_k); copy_array46_via_coupons(&mut result, src.num_registers(), |slot| src.get(slot)); + result } Mode::List { .. } | Mode::Set { .. } => { unreachable!( @@ -604,8 +604,6 @@ fn copy_or_downsample(src_mode: &Mode, src_lg_k: u8, tgt_lg_k: u8) -> Array8 { ); } } - - result } else { let mut result = Array8::new(tgt_lg_k); merge_array_with_downsample(&mut result, tgt_lg_k, src_mode, src_lg_k); diff --git a/tests-integration/tests/hll_test/union.rs b/tests-integration/tests/hll_test/union.rs index c61a1716..28aee71e 100644 --- a/tests-integration/tests/hll_test/union.rs +++ b/tests-integration/tests/hll_test/union.rs @@ -745,3 +745,31 @@ fn test_downsampling_single_hll_input_preserves_estimate() { assert_eq!(union.estimate(), sketch.estimate(), "{hll_type:?} estimate"); } } + +#[test] +fn test_copied_hll8_input_preserves_subsequent_updates() { + for lg_max_k in [10, 12] { + for merged in [false, true] { + let mut source = make_hll_sketch(HllType::Hll8, 10, 0, 2_000); + if merged { + let mut union = HllUnion::new(10).unwrap(); + union.update(&source); + union.update(&make_hll_sketch(HllType::Hll8, 10, 1_000, 3_000)); + source = union.to_sketch(HllType::Hll8); + } + let mut union = HllUnion::new(lg_max_k).unwrap(); + union.update(&source); + let mut expected = source.clone(); + + // Copying must retain both the estimate history and the caches used by later updates. + for value in 2_000..5_000_u64 { + union.update_value(value); + expected.update(value); + } + let result = union.to_sketch(HllType::Hll8); + assert_same_estimate_and_bounds(&result, &expected); + assert_eq!(result.serialize(), expected.serialize()); + assert_eq!(union.lg_config_k(), source.lg_config_k()); + } + } +} From 4c94bed7fe5fc171633f5f61d6db017607a4d10f Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 31 Oct 2026 11:47:50 +0800 Subject: [PATCH 2/9] feat(hll): serialize union results without an intermediate sketch Aggregation engines that emit an HLL union state currently call `to_sketch(HllType::Hll8).serialize()`. That first clones the union's retained storage and then copies it again into the serialized image. Add HllUnion::serialize to encode its HLL8 gadget directly. Document that the bytes represent a sketch with the effective precision, not a saved union configuration, and match the existing to_sketch serialization. Together with update_value and update, this supports aggregators that accept both raw values and partial sketches through one persistent union. Exercise empty, LIST, SET, and dense results from HLL4, HLL6, and HLL8 inputs, including reduced precision, continued updates after decoding, and reuse after reset. Compare direct serialization against the existing path in benchmarks, including allocation counts. Validation: all 16 HLL serialization tests and all 10 HLL union doctests pass. In the lg_k 18 dense-result benchmark, direct serialization removes one register-array allocation and copy, with a median of about 4 us compared with about 8 us through an intermediate sketch. --- CHANGELOG.md | 1 + benchmarks/hll/mod.rs | 1 + benchmarks/hll/serde.rs | 41 ++++++++++++++++++++++ datasketches/src/hll/union.rs | 23 ++++++++++++ tests-integration/tests/serde_tests/hll.rs | 35 ++++++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 benchmarks/hll/serde.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ebddcf1..726104ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All significant changes to this project will be documented in this file. ### New features +* Add `HllUnion::serialize` to write an HLL8 result directly, avoiding an intermediate sketch copy when emitting aggregation states. * Add immutable `XorFilter` and `XorFilterBuilder` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints, consuming builder configuration with complete-input construction, bounded seeded retries and compact temporary storage, optional cardinality metadata for imported filters, standard serialization for all three widths (Java-compatible for 8/16-bit filters), and raw parts for application-defined storage. ### Improvements diff --git a/benchmarks/hll/mod.rs b/benchmarks/hll/mod.rs index e56cecc9..9e440b27 100644 --- a/benchmarks/hll/mod.rs +++ b/benchmarks/hll/mod.rs @@ -15,5 +15,6 @@ // specific language governing permissions and limitations // under the License. +mod serde; mod union; mod update; diff --git a/benchmarks/hll/serde.rs b/benchmarks/hll/serde.rs new file mode 100644 index 00000000..8bafba49 --- /dev/null +++ b/benchmarks/hll/serde.rs @@ -0,0 +1,41 @@ +// 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. + +use datasketches::hll::HllType; +use datasketches::hll::HllUnion; +use divan::Bencher; +use divan::black_box; + +fn union(rows: u64) -> HllUnion { + let mut union = HllUnion::new(18).unwrap(); + for value in 0..rows { + union.update_value(value); + } + union +} + +#[divan::bench(args = [0, 4, 100, 80_000])] +fn serialize_union(bencher: Bencher, rows: u64) { + let union = union(rows); + bencher.bench_local(|| black_box(&union).serialize()); +} + +#[divan::bench(args = [0, 4, 100, 80_000])] +fn serialize_union_via_sketch(bencher: Bencher, rows: u64) { + let union = union(rows); + bencher.bench_local(|| black_box(&union).to_sketch(HllType::Hll8).serialize()); +} diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index 436198ee..f23f9930 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -249,6 +249,29 @@ impl HllUnion { } } + /// Serializes the union's result as an [`HllType::Hll8`] sketch. + /// + /// Produces the same bytes as `to_sketch(HllType::Hll8).serialize()` without copying an + /// intermediate sketch. The image retains the effective `lg_k` and estimate, but not the + /// union's configured maximum `lg_k`. It can be read with [`HllSketch::deserialize`]. + /// + /// # Examples + /// + /// ``` + /// use datasketches::hll::HllSketch; + /// use datasketches::hll::HllUnion; + /// + /// let mut union = HllUnion::new(12).unwrap(); + /// union.update_value("apple"); + /// let mut partial = HllSketch::deserialize(&union.serialize()).unwrap(); + /// partial.update("banana"); + /// union.update(&partial); + /// assert!((union.estimate() - 2.0).abs() < 0.01); + /// ``` + pub fn serialize(&self) -> Vec { + self.gadget.serialize() + } + /// Returns the union's current effective `lg_k`. pub fn lg_config_k(&self) -> u8 { self.gadget.lg_config_k() diff --git a/tests-integration/tests/serde_tests/hll.rs b/tests-integration/tests/serde_tests/hll.rs index 6f4485b2..ed0dd41b 100644 --- a/tests-integration/tests/serde_tests/hll.rs +++ b/tests-integration/tests/serde_tests/hll.rs @@ -21,6 +21,7 @@ use std::path::PathBuf; use datasketches::hash::value::natural_extend; use datasketches::hll::HllSketch; use datasketches::hll::HllType; +use datasketches::hll::HllUnion; use googletest::assert_that; use googletest::prelude::all; use googletest::prelude::ge; @@ -30,6 +31,40 @@ use googletest::prelude::near; use crate::serialization_test_data; +#[test] +fn union_serialization_preserves_result_and_subsequent_updates() { + for hll_type in [HllType::Hll4, HllType::Hll6, HllType::Hll8] { + for count in [0, 4, 100, 10_000_u64] { + let mut source = HllSketch::new(12, hll_type).unwrap(); + for value in 0..count { + source.update(value); + } + for lg_max_k in [8, 12, 14] { + let mut union = HllUnion::new(lg_max_k).unwrap(); + union.update(&source); + let bytes = union.serialize(); + assert_eq!(bytes, union.to_sketch(HllType::Hll8).serialize()); + + let mut restored = HllSketch::deserialize(&bytes).unwrap(); + assert_eq!(restored.target_type(), HllType::Hll8); + assert_eq!(restored.lg_config_k(), union.lg_config_k()); + assert_eq!(restored.estimate(), union.estimate()); + + for value in count..count + 1_000 { + union.update_value(value); + restored.update(value); + } + assert_eq!(union.to_sketch(HllType::Hll8), restored); + + union.reset(); + let empty = HllSketch::deserialize(&union.serialize()).unwrap(); + assert!(empty.is_empty()); + assert_eq!(empty.lg_config_k(), lg_max_k); + } + } + } +} + fn test_sketch_file(path: PathBuf, expected_cardinality: usize, expected_lg_k: u8) { let expected = expected_cardinality as f64; From b5e40bc7659337bf8626228d90c024832eda6222 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 31 Oct 2026 11:58:48 +0800 Subject: [PATCH 3/9] perf(hll): defer register cache rebuilds across dense merges Every dense merge previously scanned the resulting register array again to recompute the zero count and KxQ sums. Aggregations that merge many partials before reading a result paid that reconstruction cost after every input, even though the intermediate estimates were never used. Keep the HLL8 register-derived state in a OnceLock and invalidate it after bulk merges. An absent cache also records that HIP history is unavailable. Estimates, bounds, serialization, and equality materialize the cache when needed, preserving shared-reference queries and thread safety. Scalar updates maintain an initialized cache or keep an already invalidated cache deferred. Copying preserves cache state, while restoring a valid HIP history after downsampling rebuilds the fields needed by later updates. Check emptiness directly from registers when the cache is absent so union bookkeeping does not force a rebuild between merges. Packed HLL4/HLL6 inputs invalidate the same destination cache after their register writes. Cover mixed representations and precisions, queries interleaved with scalar updates, copying a result before its first query, and concurrent shared queries. Extend cache reconstruction coverage across the KxQ split and maximum six-bit register value. Benchmarks include a final estimate, queries after every merge, and downsampled dense partials. Validation: cargo x check, cargo x test, and cargo x lint pass. All 48 HLL integration tests also pass on Rust 1.86.0. On an Apple M4 Max, the release benchmark merging 16 dense partials at lg_k 18 and then querying the result has a median of 0.284 ms versus 4.91 ms on the base revision. Querying after every input retains the per-merge reconstruction cost, as expected. --- CHANGELOG.md | 2 +- benchmarks/hll/union.rs | 29 ++++ datasketches/src/hll/array8.rs | 158 +++++++++++++++------- datasketches/src/hll/union.rs | 4 +- tests-integration/tests/hll_test/union.rs | 85 ++++++++++++ 5 files changed, 225 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 726104ea..c260a25d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ All significant changes to this project will be documented in this file. ### Improvements -* `HllUnion` is faster when adopting an HLL8 input without downsampling, including repeated unions of a dense result with small partial sketches. +* `HllUnion` is faster when adopting HLL8 inputs without downsampling or merging multiple dense inputs before querying or serializing the result. * `KllSketch::rank` is faster for repeated queries on compacted sketches. * Repeated scalar `TDigest::rank` queries are faster, including small digests and data with repeated values. * `TDigest` and `TDigestMut` CDF and PMF batches are faster for many split points; batches with sparse split points keep the original binary-search behavior. diff --git a/benchmarks/hll/union.rs b/benchmarks/hll/union.rs index 59e5639c..c6be59c8 100644 --- a/benchmarks/hll/union.rs +++ b/benchmarks/hll/union.rs @@ -54,6 +54,7 @@ fn recreated_small_partials(bencher: Bencher, lg_k: u8) { union.update(partial); sketch = union.to_sketch(HllType::Hll8); } + black_box(sketch.estimate()); black_box(sketch) }); } @@ -66,6 +67,7 @@ fn persistent_small_partials(bencher: Bencher, lg_k: u8) { for partial in black_box(&partials) { union.update(partial); } + black_box(union.estimate()); black_box(union.to_sketch(HllType::Hll8)) }); } @@ -78,6 +80,33 @@ fn dense_partials(bencher: Bencher, lg_k: u8) { for partial in black_box(&partials) { union.update(partial); } + black_box(union.estimate()); + black_box(union.to_sketch(HllType::Hll8)) + }); +} + +#[divan::bench(args = [12, 18])] +fn dense_partials_with_queries(bencher: Bencher, lg_k: u8) { + let partials = partials(lg_k, 16, (1 << lg_k) / 4); + bencher.bench_local(|| { + let mut union = HllUnion::new(lg_k).unwrap(); + for partial in black_box(&partials) { + union.update(partial); + black_box(union.estimate()); + } + black_box(union.to_sketch(HllType::Hll8)) + }); +} + +#[divan::bench(args = [12, 18])] +fn downsampled_dense_partials(bencher: Bencher, lg_k: u8) { + let partials = partials(lg_k + 1, 16, (1 << lg_k) / 2); + bencher.bench_local(|| { + let mut union = HllUnion::new(lg_k).unwrap(); + for partial in black_box(&partials) { + union.update(partial); + } + black_box(union.estimate()); black_box(union.to_sketch(HllType::Hll8)) }); } diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index 1269d171..c111b025 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +use std::sync::OnceLock; + use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; @@ -33,14 +35,29 @@ use crate::hll::serialization::TGT_HLL8; use crate::hll::serialization::encode_mode_byte; /// Registers stored one per byte. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct Array8 { lg_config_k: u8, bytes: Box<[u8]>, + /// An absent cache means a bulk merge invalidated HIP and the register-derived fields. + /// Shared queries initialize it once; exclusive updates can maintain or invalidate it. + cache: OnceLock, +} + +#[derive(Debug, Clone, PartialEq)] +struct CachedState { num_zeros: u32, estimator: Estimator, } +impl PartialEq for Array8 { + fn eq(&self, other: &Self) -> bool { + self.lg_config_k == other.lg_config_k + && self.bytes == other.bytes + && self.cached_state() == other.cached_state() + } +} + impl Array8 { pub fn new(lg_config_k: u8) -> Self { let k = 1 << lg_config_k; @@ -48,8 +65,10 @@ impl Array8 { Self { lg_config_k, bytes: vec![0u8; k as usize].into_boxed_slice(), - num_zeros: k, - estimator: Estimator::new(lg_config_k), + cache: OnceLock::from(CachedState { + num_zeros: k, + estimator: Estimator::new(lg_config_k), + }), } } @@ -71,39 +90,54 @@ impl Array8 { let old_value = self.get(slot); if new_value > old_value { - self.estimator - .update(self.lg_config_k, old_value, new_value); - - self.put(slot, new_value); - - if old_value == 0 { - self.num_zeros -= 1; + if let Some(cache) = self.cache.get_mut() { + cache + .estimator + .update(self.lg_config_k, old_value, new_value); + if old_value == 0 { + cache.num_zeros -= 1; + } } + self.put(slot, new_value); } } pub fn estimate(&self) -> f64 { + let cache = self.cached_state(); // Array8 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros - self.estimator.estimate(self.lg_config_k, 0, self.num_zeros) + cache + .estimator + .estimate(self.lg_config_k, 0, cache.num_zeros) } pub fn composite_estimate(&self) -> f64 { - self.estimator - .composite_estimate(self.lg_config_k, 0, self.num_zeros) + let cache = self.cached_state(); + cache + .estimator + .composite_estimate(self.lg_config_k, 0, cache.num_zeros) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .upper_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) + let cache = self.cached_state(); + cache + .estimator + .upper_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .lower_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) + let cache = self.cached_state(); + cache + .estimator + .lower_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) } pub fn is_empty(&self) -> bool { - self.num_zeros == (1 << self.lg_config_k) + if let Some(cache) = self.cache.get() { + return cache.num_zeros == (1 << self.lg_config_k); + } + // Union checks emptiness between merges. A nonzero register answers that without + // rebuilding all estimator fields and defeating deferred reconstruction. + self.bytes.iter().all(|&value| value == 0) } pub fn values(&self) -> &[u8] { @@ -115,22 +149,29 @@ impl Array8 { } pub fn estimate_state(&self) -> EstimateState { - self.estimator.estimate_state() + self.cache.get().map_or(EstimateState::Composite, |cache| { + cache.estimator.estimate_state() + }) } pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimator.restore_estimate_state(state); + if let Some(cache) = self.cache.get_mut() { + cache.estimator.restore_estimate_state(state); + } else if let EstimateState::Hip(_) = state { + let mut cache = self.rebuild_cached_values(); + cache.estimator.restore_estimate_state(state); + self.cache = OnceLock::from(cache); + } } - /// Bypasses estimator updates. Call `rebuild_estimator_from_registers` after all writes. + /// Bypasses estimator updates. Call `invalidate_estimator` after all writes. pub fn set_register(&mut self, slot: usize, value: u8) { self.bytes[slot] = value; } - /// Rebuilds register-derived caches and invalidates HIP after bulk writes. - pub fn rebuild_estimator_from_registers(&mut self) { - self.rebuild_cached_values(); - self.estimator.invalidate_hip(); + /// Invalidates HIP and defers rebuilding register-derived fields until a query needs them. + pub fn invalidate_estimator(&mut self) { + self.cache.take(); } /// Merges registers and invalidates HIP because their update order is unavailable. @@ -145,8 +186,7 @@ impl Array8 { self.bytes[i] = self.bytes[i].max(val); } - self.rebuild_cached_values(); - self.estimator.invalidate_hip(); + self.invalidate_estimator(); } /// Register values come from a separate hash word, so folding slot indices needs no @@ -169,13 +209,15 @@ impl Array8 { self.bytes[dst_slot] = self.bytes[dst_slot].max(val); } - self.rebuild_cached_values(); - self.estimator.invalidate_hip(); + self.invalidate_estimator(); + } + + fn cached_state(&self) -> &CachedState { + self.cache.get_or_init(|| self.rebuild_cached_values()) } - /// Rebuilds register-derived caches without changing the estimate state. - fn rebuild_cached_values(&mut self) { - self.num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; + fn rebuild_cached_values(&self) -> CachedState { + let num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; let mut kxq0_sum = 0.0; let mut kxq1_sum = 0.0; @@ -190,7 +232,13 @@ impl Array8 { } } - self.estimator.restore_kxq(kxq0_sum, kxq1_sum); + let mut estimator = Estimator::new(self.lg_config_k); + estimator.restore_kxq(kxq0_sum, kxq1_sum); + estimator.invalidate_hip(); + CachedState { + num_zeros, + estimator, + } } pub fn deserialize_registers( @@ -236,12 +284,15 @@ impl Array8 { Ok(Self { lg_config_k, bytes: data.into_boxed_slice(), - num_zeros, - estimator, + cache: OnceLock::from(CachedState { + num_zeros, + estimator, + }), }) } pub fn serialize(&self, lg_config_k: u8) -> Vec { + let cache = self.cached_state(); let k = 1 << lg_config_k; let total_size = HLL_PREAMBLE_SIZE + k as usize; let mut bytes = SketchBytes::with_capacity(total_size); @@ -253,7 +304,7 @@ impl Array8 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if self.estimator.uses_composite_estimate() { + if cache.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -263,12 +314,12 @@ impl Array8 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL8)); - bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(self.estimator.kxq0()); - bytes.write_f64_le(self.estimator.kxq1()); + bytes.write_f64_le(cache.estimator.hip_accum()); + bytes.write_f64_le(cache.estimator.kxq0()); + bytes.write_f64_le(cache.estimator.kxq1()); // Write num_at_cur_min (num_zeros for Array8) - bytes.write_u32_le(self.num_zeros); + bytes.write_u32_le(cache.num_zeros); // Write aux_count (always 0 for Array8) bytes.write_u32_le(0); @@ -398,11 +449,11 @@ mod tests { arr.update(Coupon::pack(1, 50)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.estimator.kxq0(), lt(256.0)); + assert_that!(arr.cached_state().estimator.kxq0(), lt(256.0)); // kxq1 should have a positive value (from 1/2^50) - assert_that!(arr.estimator.kxq1(), gt(0.0)); - assert_that!(arr.estimator.kxq1(), lt(1e-10)); + assert_that!(arr.cached_state().estimator.kxq1(), gt(0.0)); + assert_that!(arr.cached_state().estimator.kxq1(), lt(1e-10)); } #[test] @@ -442,9 +493,9 @@ mod tests { assert_eq!(dst.get(3), 40, "dst[3] got new value"); // Bulk merges require composite estimation. - assert!(dst.estimator.uses_composite_estimate()); + assert!(dst.cached_state().estimator.uses_composite_estimate()); - assert_eq!(dst.num_zeros, 12); + assert_eq!(dst.cached_state().num_zeros, 12); } #[test] @@ -468,7 +519,7 @@ mod tests { assert_eq!(dst.get(1), 30, "dst[1] = max(20, 18, 30)"); // Bulk merges require composite estimation. - assert!(dst.estimator.uses_composite_estimate()); + assert!(dst.cached_state().estimator.uses_composite_estimate()); } #[test] @@ -497,11 +548,18 @@ mod tests { arr.put(1, 20); arr.put(2, 30); - arr.num_zeros = 999; + arr.put(3, 31); + arr.put(4, 32); + arr.put(5, 63); + arr.invalidate_estimator(); - arr.rebuild_cached_values(); - - assert_eq!(arr.num_zeros, 13); + let cache = arr.cached_state(); + assert_eq!(cache.num_zeros, 10); + assert_eq!( + cache.estimator.kxq0(), + 10.0 + 2_f64.powi(-10) + 2_f64.powi(-20) + 2_f64.powi(-30) + 2_f64.powi(-31) + ); + assert_eq!(cache.estimator.kxq1(), 2_f64.powi(-32) + 2_f64.powi(-63)); } #[test] diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index f23f9930..cef8bf9f 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -491,7 +491,7 @@ fn merge_array46_same_lgk(dst: &mut Array8, num_registers: usize, get_value: imp dst.set_register(slot, val); } } - dst.rebuild_estimator_from_registers(); + dst.invalidate_estimator(); } /// Merging independent register histories invalidates the HIP accumulator. @@ -529,7 +529,7 @@ fn merge_array46_with_downsample( } } } - dst.rebuild_estimator_from_registers(); + dst.invalidate_estimator(); } /// Multiple source registers map to each destination register via masking. diff --git a/tests-integration/tests/hll_test/union.rs b/tests-integration/tests/hll_test/union.rs index 28aee71e..5f807d8e 100644 --- a/tests-integration/tests/hll_test/union.rs +++ b/tests-integration/tests/hll_test/union.rs @@ -773,3 +773,88 @@ fn test_copied_hll8_input_preserves_subsequent_updates() { } } } + +#[test] +fn test_union_query_timing_does_not_change_mixed_updates() { + let mut queried = HllUnion::new(12).unwrap(); + let mut deferred = queried.clone(); + let inputs = [ + (HllType::Hll8, 12), + (HllType::Hll4, 12), + (HllType::Hll6, 14), + (HllType::Hll8, 10), + (HllType::Hll4, 8), + ]; + for (index, (hll_type, lg_k)) in inputs.into_iter().enumerate() { + let start = index as u64 * 10_000; + let source = make_hll_sketch(hll_type, lg_k, start, start + 5_000); + queried.update(&source); + deferred.update(&source); + assert!(!deferred.is_empty()); + + // Queries refresh one accumulator between merges; the other keeps accepting updates. + assert_same_estimate_and_bounds( + &queried.to_sketch(HllType::Hll8), + &queried.to_sketch(HllType::Hll4), + ); + let restored = HllSketch::deserialize(&queried.serialize()).unwrap(); + assert_eq!(restored.composite_estimate(), queried.composite_estimate()); + for value in start + 5_000..start + 5_100 { + queried.update_value(value); + deferred.update_value(value); + } + } + + assert_eq!(queried.serialize(), deferred.serialize()); + assert_same_estimate_and_bounds( + &queried.to_sketch(HllType::Hll8), + &deferred.to_sketch(HllType::Hll8), + ); + + // A copied result can be merged again before either its source or the copy is queried. + let source = make_hll_sketch(HllType::Hll8, 8, 50_000, 55_000); + deferred.update(&source); + let result = deferred.to_sketch(HllType::Hll8); + let mut copied = HllUnion::new(12).unwrap(); + copied.update(&result); + for value in 55_000..56_000_u64 { + copied.update_value(value); + deferred.update_value(value); + } + assert_eq!(copied.serialize(), deferred.serialize()); +} + +#[test] +fn test_union_supports_shared_queries_after_dense_merges() { + let mut union = HllUnion::new(12).unwrap(); + for index in 0..3 { + union.update(&make_hll_sketch( + HllType::Hll8, + 12, + index * 5_000, + (index + 1) * 5_000, + )); + } + let expected = union.to_sketch(HllType::Hll8); + let bytes = expected.serialize(); + let barrier = std::sync::Barrier::new(4); + + std::thread::scope(|scope| { + for _ in 0..4 { + scope.spawn(|| { + barrier.wait(); + assert_eq!(union.estimate(), expected.estimate()); + assert_eq!(union.composite_estimate(), expected.composite_estimate()); + assert_eq!( + union.lower_bound(NumStdDev::Two), + expected.lower_bound(NumStdDev::Two) + ); + assert_eq!( + union.upper_bound(NumStdDev::Two), + expected.upper_bound(NumStdDev::Two) + ); + assert_eq!(union.serialize(), bytes); + }); + } + }); +} From fb2c3602d51a1079fafb7dd45b6814ac02800ba0 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 09:58:34 +0800 Subject: [PATCH 4/9] docs: describe direct HLL union serialization in the changelog Describe the new public capability without register-encoding details or intermediate-copy mechanics. Keep the output-format contract in the API documentation, where callers can find it when choosing how to serialize a union result. Validation: cargo x lint passes in a clean worktree containing the same change; git diff --check passes. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c260a25d..8f254ac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All significant changes to this project will be documented in this file. ### New features -* Add `HllUnion::serialize` to write an HLL8 result directly, avoiding an intermediate sketch copy when emitting aggregation states. +* Add `HllUnion::serialize` to serialize union results directly. * Add immutable `XorFilter` and `XorFilterBuilder` behind the `xor` feature, with 8-, 16-, and 32-bit fingerprints, consuming builder configuration with complete-input construction, bounded seeded retries and compact temporary storage, optional cardinality metadata for imported filters, standard serialization for all three widths (Java-compatible for 8/16-bit filters), and raw parts for application-defined storage. ### Improvements From 6eb6363d12f2b3f38e7aefd703abba474d2c1b4b Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 10:34:01 +0800 Subject: [PATCH 5/9] refactor(hll): centralize estimation history and register statistics Dense unions need to defer rebuilding register-derived statistics across merges without treating HIP update history as reconstructible cache data. Keep that distinction in the shared Estimator instead of an Array8-specific CachedState wrapper. Store estimate history in state and keep zero counts and KxQ sums together in RegisterStatistics. Estimator owns their lazy initialization, invalidation, scalar maintenance, and HIP restoration for all three HLL representations. OnceLock preserves shared queries without requiring mutable access. HLL4 retains its packing minimum and minimum count as storage metadata, independently of the logical zero-register count. Remove Array8's cache lifecycle handling and Array6's separate zero count. Keep scalar update paths inline and expand their benchmarks to cover HLL4, HLL6, and HLL8. Add coverage for restoring HIP after deferred reconstruction and for estimates across HLL4 minimum shifts and serialization round trips. On aarch64, the shared statistics increase HllSketch's inline size from 88 to 104 bytes. The serialized format is unchanged. A benchmark of 16 dense merges at lg_k=18 followed by a query retains deferred reconstruction, with median time decreasing from approximately 288 us to 252 us. Validation: cargo x check, cargo x test, cargo x lint, cross-language serialization fixtures, Rust 1.86 HLL tests, and HLL update/union benchmarks. --- benchmarks/hll/update.rs | 6 +- datasketches/src/hll/array4.rs | 84 +++-- datasketches/src/hll/array6.rs | 71 +++-- datasketches/src/hll/array8.rs | 160 ++++------ datasketches/src/hll/estimator.rs | 313 ++++++++++++------- tests-integration/tests/hll_test/estimate.rs | 39 +++ 6 files changed, 386 insertions(+), 287 deletions(-) diff --git a/benchmarks/hll/update.rs b/benchmarks/hll/update.rs index 88a335c1..ca38bcc0 100644 --- a/benchmarks/hll/update.rs +++ b/benchmarks/hll/update.rs @@ -25,10 +25,10 @@ use divan::counter::ItemsCount; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; -#[divan::bench] -fn u64(bencher: Bencher) { +#[divan::bench(args = [HllType::Hll4, HllType::Hll6, HllType::Hll8])] +fn u64(bencher: Bencher, hll_type: HllType) { bencher.counter(ItemsCount::new(ITEMS)).bench_local(|| { - let mut sketch = HllSketch::new(12, HllType::Hll8).unwrap(); + let mut sketch = HllSketch::new(12, hll_type).unwrap(); for value in 0..ITEMS as u64 { sketch.update(black_box(value)); } diff --git a/datasketches/src/hll/array4.rs b/datasketches/src/hll/array4.rs index 4190a3f7..309d0e4c 100644 --- a/datasketches/src/hll/array4.rs +++ b/datasketches/src/hll/array4.rs @@ -90,16 +90,7 @@ impl Array4 { } pub fn get(&self, slot: u32) -> u8 { - let raw = self.get_raw(slot); - - if raw < AUX_TOKEN { - self.cur_min + raw - } else { - self.aux_map - .as_ref() - .and_then(|map| map.get(slot)) - .unwrap_or(self.cur_min) - } + read_register(&self.bytes, self.cur_min, self.aux_map.as_ref(), slot) } pub fn num_registers(&self) -> usize { @@ -236,40 +227,42 @@ impl Array4 { self.num_at_cur_min = num_at_new; } + fn register_values(&self) -> impl Iterator { + (0..self.num_registers() as u32).map(|slot| self.get(slot)) + } + pub fn estimate(&self) -> f64 { self.estimator - .estimate(self.lg_config_k, self.cur_min, self.num_at_cur_min) + .estimate(self.lg_config_k, self.register_values()) } pub fn composite_estimate(&self) -> f64 { self.estimator - .composite_estimate(self.lg_config_k, self.cur_min, self.num_at_cur_min) + .composite_estimate(self.lg_config_k, self.register_values()) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator.upper_bound( - self.lg_config_k, - self.cur_min, - self.num_at_cur_min, - num_std_dev, - ) + self.estimator + .upper_bound(self.lg_config_k, self.register_values(), num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator.lower_bound( - self.lg_config_k, - self.cur_min, - self.num_at_cur_min, - num_std_dev, - ) + self.estimator + .lower_bound(self.lg_config_k, self.register_values(), num_std_dev) } pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimator.restore_estimate_state(state); + let bytes = &self.bytes; + let cur_min = self.cur_min; + let aux_map = self.aux_map.as_ref(); + let registers = + (0..1 << self.lg_config_k).map(|slot| read_register(bytes, cur_min, aux_map, slot)); + self.estimator.restore_estimate_state(state, registers); } pub fn is_empty(&self) -> bool { - self.num_at_cur_min == (1 << self.lg_config_k) && self.cur_min == 0 + self.estimator + .is_empty(self.lg_config_k, self.register_values()) } pub fn deserialize( @@ -360,7 +353,13 @@ impl Array4 { aux_map = Some(aux); } - let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + let estimator = Estimator::from_serialized( + hip_accum, + kxq0, + kxq1, + if cur_min == 0 { num_at_cur_min } else { 0 }, + ooo, + ); Ok(Self { lg_config_k, @@ -374,6 +373,7 @@ impl Array4 { /// Writes packed registers followed by compact auxiliary entries. pub fn serialize(&self, lg_config_k: u8) -> Vec { + let statistics = self.estimator.statistics(self.register_values()); let num_bytes = 1 << (lg_config_k - 1); let aux_entries: Vec<(u32, u8)> = if let Some(aux) = &self.aux_map { @@ -403,8 +403,8 @@ impl Array4 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL4)); bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(self.estimator.kxq0()); - bytes.write_f64_le(self.estimator.kxq1()); + bytes.write_f64_le(statistics.kxq0); + bytes.write_f64_le(statistics.kxq1); bytes.write_u32_le(self.num_at_cur_min); @@ -430,6 +430,17 @@ impl Array4 { } } +fn read_register(bytes: &[u8], cur_min: u8, aux_map: Option<&AuxMap>, slot: u32) -> u8 { + let byte = bytes[(slot >> 1) as usize]; + let raw = if slot & 1 == 0 { byte & 15 } else { byte >> 4 }; + + if raw < AUX_TOKEN { + cur_min + raw + } else { + aux_map.and_then(|map| map.get(slot)).unwrap_or(cur_min) + } +} + #[cfg(test)] mod tests { use googletest::assert_that; @@ -491,11 +502,20 @@ mod tests { // kxq0 should have decreased (we removed a 0 and added a 10) // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.estimator.kxq0(), lt(256.0)); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq0, + lt(256.0) + ); // kxq1 should have a small positive value (from 1/2^40) - assert_that!(arr.estimator.kxq1(), gt(0.0)); - assert_that!(arr.estimator.kxq1(), lt(0.001)); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq1, + gt(0.0) + ); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq1, + lt(0.001) + ); } #[test] diff --git a/datasketches/src/hll/array6.rs b/datasketches/src/hll/array6.rs index 636da15d..74b89d39 100644 --- a/datasketches/src/hll/array6.rs +++ b/datasketches/src/hll/array6.rs @@ -39,7 +39,6 @@ const VAL_MASK_6: u16 = 0x3F; pub struct Array6 { lg_config_k: u8, bytes: Box<[u8]>, - num_zeros: u32, estimator: Estimator, } @@ -51,20 +50,13 @@ impl Array6 { Self { lg_config_k, bytes: vec![0u8; num_bytes].into_boxed_slice(), - num_zeros: k, estimator: Estimator::new(lg_config_k), } } #[inline] fn get_raw(&self, slot: u32) -> u8 { - let start_bit = slot * 6; - let byte_idx = (start_bit >> 3) as usize; - let shift = (start_bit & 7) as u8; - - let two_bytes = u16::from_le_bytes([self.bytes[byte_idx], self.bytes[byte_idx + 1]]); - - ((two_bytes >> shift) & VAL_MASK_6) as u8 + read_register(&self.bytes, slot) } #[inline] @@ -99,6 +91,7 @@ impl Array6 { self.bytes[byte_idx + 1] = bytes_out[1]; } + #[inline] pub fn update(&mut self, coupon: Coupon) { let mask = (1 << self.lg_config_k) - 1; let slot = coupon.slot() & mask; @@ -111,39 +104,42 @@ impl Array6 { .update(self.lg_config_k, old_value, new_value); self.put_raw(slot, new_value); - - if old_value == 0 { - self.num_zeros -= 1; - } } } + fn register_values(&self) -> impl Iterator { + (0..self.num_registers() as u32).map(|slot| self.get(slot)) + } + pub fn estimate(&self) -> f64 { - // Array6 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros - self.estimator.estimate(self.lg_config_k, 0, self.num_zeros) + self.estimator + .estimate(self.lg_config_k, self.register_values()) } pub fn composite_estimate(&self) -> f64 { self.estimator - .composite_estimate(self.lg_config_k, 0, self.num_zeros) + .composite_estimate(self.lg_config_k, self.register_values()) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { self.estimator - .upper_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) + .upper_bound(self.lg_config_k, self.register_values(), num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { self.estimator - .lower_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) + .lower_bound(self.lg_config_k, self.register_values(), num_std_dev) } pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimator.restore_estimate_state(state); + let bytes = &self.bytes; + let registers = (0..1 << self.lg_config_k).map(|slot| read_register(bytes, slot)); + self.estimator.restore_estimate_state(state, registers); } pub fn is_empty(&self) -> bool { - self.num_zeros == (1 << self.lg_config_k) + self.estimator + .is_empty(self.lg_config_k, self.register_values()) } pub fn deserialize_registers( @@ -185,17 +181,17 @@ impl Array6 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, num_zeros, ooo); Ok(Self { lg_config_k, bytes: data.into_boxed_slice(), - num_zeros, estimator, }) } pub fn serialize(&self, lg_config_k: u8) -> Vec { + let statistics = self.estimator.statistics(self.register_values()); let k = 1 << lg_config_k; let num_bytes = num_bytes_for_k(k); let total_size = HLL_PREAMBLE_SIZE + num_bytes; @@ -219,11 +215,11 @@ impl Array6 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL6)); bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(self.estimator.kxq0()); - bytes.write_f64_le(self.estimator.kxq1()); + bytes.write_f64_le(statistics.kxq0); + bytes.write_f64_le(statistics.kxq1); // Write num_at_cur_min (num_zeros for Array6) - bytes.write_u32_le(self.num_zeros); + bytes.write_u32_le(statistics.num_zeros); // Write aux_count (always 0 for Array6) bytes.write_u32_le(0); @@ -239,6 +235,16 @@ impl Array6 { } } +fn read_register(bytes: &[u8], slot: u32) -> u8 { + let start_bit = slot * 6; + let byte_idx = (start_bit >> 3) as usize; + let shift = (start_bit & 7) as u8; + + let two_bytes = u16::from_le_bytes([bytes[byte_idx], bytes[byte_idx + 1]]); + + ((two_bytes >> shift) & VAL_MASK_6) as u8 +} + fn num_bytes_for_k(k: u32) -> usize { // The final register's 16-bit read/write window needs one padding byte. (((k * 3) >> 2) + 1) as usize @@ -349,10 +355,19 @@ mod tests { arr.update(Coupon::pack(1, 40)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.estimator.kxq0(), lt(256.0)); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq0, + lt(256.0) + ); // kxq1 should have a small positive value (from 1/2^40) - assert_that!(arr.estimator.kxq1(), gt(0.0)); - assert_that!(arr.estimator.kxq1(), lt(0.001)); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq1, + gt(0.0) + ); + assert_that!( + arr.estimator.statistics(arr.register_values()).kxq1, + lt(0.001) + ); } } diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index c111b025..04037a65 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -use std::sync::OnceLock; - use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; @@ -39,14 +37,6 @@ use crate::hll::serialization::encode_mode_byte; pub struct Array8 { lg_config_k: u8, bytes: Box<[u8]>, - /// An absent cache means a bulk merge invalidated HIP and the register-derived fields. - /// Shared queries initialize it once; exclusive updates can maintain or invalidate it. - cache: OnceLock, -} - -#[derive(Debug, Clone, PartialEq)] -struct CachedState { - num_zeros: u32, estimator: Estimator, } @@ -54,7 +44,9 @@ impl PartialEq for Array8 { fn eq(&self, other: &Self) -> bool { self.lg_config_k == other.lg_config_k && self.bytes == other.bytes - && self.cached_state() == other.cached_state() + && self.estimator.estimate_state() == other.estimator.estimate_state() + && self.estimator.statistics(self.bytes.iter().copied()) + == other.estimator.statistics(other.bytes.iter().copied()) } } @@ -65,10 +57,7 @@ impl Array8 { Self { lg_config_k, bytes: vec![0u8; k as usize].into_boxed_slice(), - cache: OnceLock::from(CachedState { - num_zeros: k, - estimator: Estimator::new(lg_config_k), - }), + estimator: Estimator::new(lg_config_k), } } @@ -82,6 +71,7 @@ impl Array8 { self.bytes[slot as usize] = value; } + #[inline] pub fn update(&mut self, coupon: Coupon) { let mask = (1 << self.lg_config_k) - 1; let slot = coupon.slot() & mask; @@ -90,54 +80,35 @@ impl Array8 { let old_value = self.get(slot); if new_value > old_value { - if let Some(cache) = self.cache.get_mut() { - cache - .estimator - .update(self.lg_config_k, old_value, new_value); - if old_value == 0 { - cache.num_zeros -= 1; - } - } + self.estimator + .update(self.lg_config_k, old_value, new_value); self.put(slot, new_value); } } pub fn estimate(&self) -> f64 { - let cache = self.cached_state(); - // Array8 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros - cache - .estimator - .estimate(self.lg_config_k, 0, cache.num_zeros) + self.estimator + .estimate(self.lg_config_k, self.bytes.iter().copied()) } pub fn composite_estimate(&self) -> f64 { - let cache = self.cached_state(); - cache - .estimator - .composite_estimate(self.lg_config_k, 0, cache.num_zeros) + self.estimator + .composite_estimate(self.lg_config_k, self.bytes.iter().copied()) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - let cache = self.cached_state(); - cache - .estimator - .upper_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + self.estimator + .upper_bound(self.lg_config_k, self.bytes.iter().copied(), num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - let cache = self.cached_state(); - cache - .estimator - .lower_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + self.estimator + .lower_bound(self.lg_config_k, self.bytes.iter().copied(), num_std_dev) } pub fn is_empty(&self) -> bool { - if let Some(cache) = self.cache.get() { - return cache.num_zeros == (1 << self.lg_config_k); - } - // Union checks emptiness between merges. A nonzero register answers that without - // rebuilding all estimator fields and defeating deferred reconstruction. - self.bytes.iter().all(|&value| value == 0) + self.estimator + .is_empty(self.lg_config_k, self.bytes.iter().copied()) } pub fn values(&self) -> &[u8] { @@ -149,19 +120,12 @@ impl Array8 { } pub fn estimate_state(&self) -> EstimateState { - self.cache.get().map_or(EstimateState::Composite, |cache| { - cache.estimator.estimate_state() - }) + self.estimator.estimate_state() } pub fn restore_estimate_state(&mut self, state: EstimateState) { - if let Some(cache) = self.cache.get_mut() { - cache.estimator.restore_estimate_state(state); - } else if let EstimateState::Hip(_) = state { - let mut cache = self.rebuild_cached_values(); - cache.estimator.restore_estimate_state(state); - self.cache = OnceLock::from(cache); - } + self.estimator + .restore_estimate_state(state, self.bytes.iter().copied()); } /// Bypasses estimator updates. Call `invalidate_estimator` after all writes. @@ -171,7 +135,7 @@ impl Array8 { /// Invalidates HIP and defers rebuilding register-derived fields until a query needs them. pub fn invalidate_estimator(&mut self) { - self.cache.take(); + self.estimator.invalidate(); } /// Merges registers and invalidates HIP because their update order is unavailable. @@ -212,35 +176,6 @@ impl Array8 { self.invalidate_estimator(); } - fn cached_state(&self) -> &CachedState { - self.cache.get_or_init(|| self.rebuild_cached_values()) - } - - fn rebuild_cached_values(&self) -> CachedState { - let num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; - - let mut kxq0_sum = 0.0; - let mut kxq1_sum = 0.0; - - for &val in self.bytes.iter() { - if val == 0 { - kxq0_sum += 1.0; - } else if val < 32 { - kxq0_sum += 1.0 / (1u64 << val) as f64; - } else { - kxq1_sum += 1.0 / (1u64 << val) as f64; - } - } - - let mut estimator = Estimator::new(self.lg_config_k); - estimator.restore_kxq(kxq0_sum, kxq1_sum); - estimator.invalidate_hip(); - CachedState { - num_zeros, - estimator, - } - } - pub fn deserialize_registers( mut cursor: SketchSlice, lg_config_k: u8, @@ -279,20 +214,17 @@ impl Array8 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, num_zeros, ooo); Ok(Self { lg_config_k, bytes: data.into_boxed_slice(), - cache: OnceLock::from(CachedState { - num_zeros, - estimator, - }), + estimator, }) } pub fn serialize(&self, lg_config_k: u8) -> Vec { - let cache = self.cached_state(); + let statistics = self.estimator.statistics(self.bytes.iter().copied()); let k = 1 << lg_config_k; let total_size = HLL_PREAMBLE_SIZE + k as usize; let mut bytes = SketchBytes::with_capacity(total_size); @@ -304,7 +236,7 @@ impl Array8 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if cache.estimator.uses_composite_estimate() { + if self.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -314,12 +246,12 @@ impl Array8 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL8)); - bytes.write_f64_le(cache.estimator.hip_accum()); - bytes.write_f64_le(cache.estimator.kxq0()); - bytes.write_f64_le(cache.estimator.kxq1()); + bytes.write_f64_le(self.estimator.hip_accum()); + bytes.write_f64_le(statistics.kxq0); + bytes.write_f64_le(statistics.kxq1); // Write num_at_cur_min (num_zeros for Array8) - bytes.write_u32_le(cache.num_zeros); + bytes.write_u32_le(statistics.num_zeros); // Write aux_count (always 0 for Array8) bytes.write_u32_le(0); @@ -449,11 +381,20 @@ mod tests { arr.update(Coupon::pack(1, 50)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.cached_state().estimator.kxq0(), lt(256.0)); + assert_that!( + arr.estimator.statistics(arr.bytes.iter().copied()).kxq0, + lt(256.0) + ); // kxq1 should have a positive value (from 1/2^50) - assert_that!(arr.cached_state().estimator.kxq1(), gt(0.0)); - assert_that!(arr.cached_state().estimator.kxq1(), lt(1e-10)); + assert_that!( + arr.estimator.statistics(arr.bytes.iter().copied()).kxq1, + gt(0.0) + ); + assert_that!( + arr.estimator.statistics(arr.bytes.iter().copied()).kxq1, + lt(1e-10) + ); } #[test] @@ -493,9 +434,14 @@ mod tests { assert_eq!(dst.get(3), 40, "dst[3] got new value"); // Bulk merges require composite estimation. - assert!(dst.cached_state().estimator.uses_composite_estimate()); + assert!(dst.estimator.uses_composite_estimate()); - assert_eq!(dst.cached_state().num_zeros, 12); + assert_eq!( + dst.estimator + .statistics(dst.bytes.iter().copied()) + .num_zeros, + 12 + ); } #[test] @@ -519,7 +465,7 @@ mod tests { assert_eq!(dst.get(1), 30, "dst[1] = max(20, 18, 30)"); // Bulk merges require composite estimation. - assert!(dst.cached_state().estimator.uses_composite_estimate()); + assert!(dst.estimator.uses_composite_estimate()); } #[test] @@ -553,13 +499,13 @@ mod tests { arr.put(5, 63); arr.invalidate_estimator(); - let cache = arr.cached_state(); - assert_eq!(cache.num_zeros, 10); + let statistics = arr.estimator.statistics(arr.bytes.iter().copied()); + assert_eq!(statistics.num_zeros, 10); assert_eq!( - cache.estimator.kxq0(), + statistics.kxq0, 10.0 + 2_f64.powi(-10) + 2_f64.powi(-20) + 2_f64.powi(-30) + 2_f64.powi(-31) ); - assert_eq!(cache.estimator.kxq1(), 2_f64.powi(-32) + 2_f64.powi(-63)); + assert_eq!(statistics.kxq1, 2_f64.powi(-32) + 2_f64.powi(-63)); } #[test] diff --git a/datasketches/src/hll/estimator.rs b/datasketches/src/hll/estimator.rs index a72fcca0..0141dd97 100644 --- a/datasketches/src/hll/estimator.rs +++ b/datasketches/src/hll/estimator.rs @@ -23,6 +23,8 @@ //! Estimator formulas and crossover constants follow datasketches-cpp: //! +use std::sync::OnceLock; + use crate::common::NumStdDev; use crate::common::inv_pow2::inv_pow2; use crate::hll::composite_interpolation; @@ -38,87 +40,100 @@ pub enum EstimateState { Composite, } -/// KxQ caches the sum of `2^-register`. Splitting the sum at register value 32 preserves -/// precision when large and small contributions coexist. +/// Estimation history and lazily materialized statistics of the logical registers. +/// +/// HIP depends on update order and cannot be reconstructed from register values. Bulk merges +/// discard that history and invalidate only the derived statistics. Queries share a single +/// reconstruction until the next bulk merge; scalar updates maintain materialized statistics. #[derive(Debug, Clone, PartialEq)] pub struct Estimator { - estimate_state: EstimateState, - /// KxQ register for values < 32 (larger inverse powers) - kxq0: f64, - /// KxQ register for values >= 32 (tiny inverse powers) - kxq1: f64, + state: EstimateState, + statistics: OnceLock, +} + +/// Representation-independent statistics derived entirely from the current register values. +/// +/// Splitting the sum of `2^-register` at value 32 preserves precision when large and small +/// contributions coexist. HLL4's packing minimum and its count are separate storage metadata. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct RegisterStatistics { + pub num_zeros: u32, + pub kxq0: f64, + pub kxq1: f64, } impl Estimator { pub fn new(lg_config_k: u8) -> Self { let k = 1 << lg_config_k; Self { - estimate_state: EstimateState::Hip(0.0), - kxq0: k as f64, // All registers start at 0, so kxq0 = k * (1/2^0) = k - kxq1: 0.0, + state: EstimateState::Hip(0.0), + statistics: OnceLock::from(RegisterStatistics { + num_zeros: k, + kxq0: k as f64, + kxq1: 0.0, + }), } } - /// Restores estimator fields from an HLL serialization preamble. - pub fn from_serialized(hip_accum: f64, kxq0: f64, kxq1: f64, out_of_order: bool) -> Self { - let estimate_state = if out_of_order { - EstimateState::Composite - } else { - EstimateState::Hip(hip_accum) - }; + /// Restores history and statistics from an HLL serialization preamble. + pub fn from_serialized( + hip_accum: f64, + kxq0: f64, + kxq1: f64, + num_zeros: u32, + out_of_order: bool, + ) -> Self { Self { - estimate_state, - kxq0, - kxq1, + state: if out_of_order { + EstimateState::Composite + } else { + EstimateState::Hip(hip_accum) + }, + statistics: OnceLock::from(RegisterStatistics { + num_zeros, + kxq0, + kxq1, + }), } } /// Applies a register increase; requires `new_value > old_value`. + #[inline] pub fn update(&mut self, lg_config_k: u8, old_value: u8, new_value: u8) { - let k = (1 << lg_config_k) as f64; - - // HIP must use the KxQ sums from before this register update. - if let EstimateState::Hip(hip_accum) = &mut self.estimate_state { - *hip_accum += k / (self.kxq0 + self.kxq1); - } - - // Always update KxQ because it depends only on the current registers. - self.update_kxq(old_value, new_value); - } - - fn update_kxq(&mut self, old_value: u8, new_value: u8) { - if old_value < 32 { - self.kxq0 -= inv_pow2(old_value); + if let Some(statistics) = self.statistics.get_mut() { + // HIP must use the sums from before this register update. + if let EstimateState::Hip(hip_accum) = &mut self.state { + *hip_accum += (1 << lg_config_k) as f64 / (statistics.kxq0 + statistics.kxq1); + } + statistics.update(old_value, new_value); } else { - self.kxq1 -= inv_pow2(old_value); + debug_assert_eq!(self.state, EstimateState::Composite); } + } - if new_value < 32 { - self.kxq0 += inv_pow2(new_value); - } else { - self.kxq1 += inv_pow2(new_value); - } + pub fn statistics(&self, registers: impl Iterator) -> &RegisterStatistics { + self.statistics + .get_or_init(|| RegisterStatistics::from_registers(registers)) } - /// For `Array6`/`Array8`, pass zero for `cur_min` and the zero-register count for - /// `num_at_cur_min`. `Array4` tracks its actual minimum and its count. - pub fn estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { - match self.estimate_state { + pub fn estimate(&self, lg_config_k: u8, registers: impl Iterator) -> f64 { + match self.state { EstimateState::Hip(hip_accum) => hip_accum, - EstimateState::Composite => { - self.composite_estimate(lg_config_k, cur_min, num_at_cur_min) - } + EstimateState::Composite => self.composite_estimate(lg_config_k, registers), } } + pub fn composite_estimate(&self, lg_config_k: u8, registers: impl Iterator) -> f64 { + self.statistics(registers).composite_estimate(lg_config_k) + } + pub fn upper_bound( &self, lg_config_k: u8, - cur_min: u8, - num_at_cur_min: u32, + registers: impl Iterator, num_std_dev: NumStdDev, ) -> f64 { - let estimate = self.estimate(lg_config_k, cur_min, num_at_cur_min); + let estimate = self.estimate(lg_config_k, registers); let rse = relative_error( lg_config_k, true, @@ -128,29 +143,105 @@ impl Estimator { estimate / (1.0 + rse) } - /// Each nonzero register requires a distinct item, so their count is a hard floor - /// for the lower bound. If `cur_min` is nonzero, all `k` registers are nonzero. pub fn lower_bound( &self, lg_config_k: u8, - cur_min: u8, - num_at_cur_min: u32, + registers: impl Iterator, num_std_dev: NumStdDev, ) -> f64 { - let estimate = self.estimate(lg_config_k, cur_min, num_at_cur_min); + let statistics = self.statistics(registers); + let estimate = match self.state { + EstimateState::Hip(hip_accum) => hip_accum, + EstimateState::Composite => statistics.composite_estimate(lg_config_k), + }; let rse = relative_error( lg_config_k, false, self.uses_composite_estimate(), num_std_dev, ); - let config_k = 1u32 << lg_config_k; - let num_nonzero_registers = if cur_min == 0 { - config_k - num_at_cur_min + // Each nonzero register requires at least one distinct item. + (estimate / (1.0 + rse)).max(f64::from((1 << lg_config_k) - statistics.num_zeros)) + } + + pub fn is_empty(&self, lg_config_k: u8, mut registers: impl Iterator) -> bool { + if let Some(statistics) = self.statistics.get() { + statistics.num_zeros == (1 << lg_config_k) } else { - config_k + // Union checks emptiness between merges; stop at the first nonzero register + // without materializing statistics that the next merge would invalidate. + registers.all(|value| value == 0) + } + } + + pub fn hip_accum(&self) -> f64 { + match self.state { + EstimateState::Hip(hip_accum) => hip_accum, + EstimateState::Composite => 0.0, + } + } + + pub fn uses_composite_estimate(&self) -> bool { + matches!(self.state, EstimateState::Composite) + } + + pub fn estimate_state(&self) -> EstimateState { + self.state + } + + /// Restores history after copying or transforming the same logical sketch. + pub fn restore_estimate_state( + &mut self, + state: EstimateState, + registers: impl Iterator, + ) { + if let EstimateState::Hip(_) = state { + // Subsequent scalar updates need the sums to advance the restored HIP accumulator. + self.statistics(registers); + } + self.state = state; + } + + /// Discards update history and defers rebuilding statistics after a bulk register write. + pub fn invalidate(&mut self) { + self.state = EstimateState::Composite; + self.statistics.take(); + } +} + +impl RegisterStatistics { + fn from_registers(registers: impl Iterator) -> Self { + let mut statistics = Self { + num_zeros: 0, + kxq0: 0.0, + kxq1: 0.0, }; - (estimate / (1.0 + rse)).max(f64::from(num_nonzero_registers)) + for value in registers { + if value == 0 { + statistics.num_zeros += 1; + statistics.kxq0 += 1.0; + } else if value < 32 { + statistics.kxq0 += 1.0 / (1u64 << value) as f64; + } else { + statistics.kxq1 += 1.0 / (1u64 << value) as f64; + } + } + statistics + } + + #[inline] + fn update(&mut self, old_value: u8, new_value: u8) { + self.num_zeros -= u32::from(old_value == 0); + if old_value < 32 { + self.kxq0 -= inv_pow2(old_value); + } else { + self.kxq1 -= inv_pow2(old_value); + } + if new_value < 32 { + self.kxq0 += inv_pow2(new_value); + } else { + self.kxq1 += inv_pow2(new_value); + } } fn raw_estimate(&self, lg_config_k: u8) -> f64 { @@ -167,10 +258,10 @@ impl Estimator { (correction_factor * k * k) / (self.kxq0 + self.kxq1) } - fn bitmap_estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { + fn bitmap_estimate(&self, lg_config_k: u8) -> f64 { let k = 1 << lg_config_k; - let num_unhit = if cur_min == 0 { num_at_cur_min } else { 0 }; + let num_unhit = self.num_zeros; // Preserve the upstream fallback for a bitmap with no empty registers. if num_unhit == 0 { @@ -182,7 +273,7 @@ impl Estimator { } /// Selects between bias-corrected HLL and bitmap estimates using their empirical crossover. - pub fn composite_estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { + pub fn composite_estimate(&self, lg_config_k: u8) -> f64 { let raw_estimate = self.raw_estimate(lg_config_k); let x_arr = composite_interpolation::get_x_arr(lg_config_k); @@ -212,7 +303,7 @@ impl Estimator { return adjusted_estimate; } - let linear_estimate = self.bitmap_estimate(lg_config_k, cur_min, num_at_cur_min); + let linear_estimate = self.bitmap_estimate(lg_config_k); // Comparing the average to the threshold reduces estimator-selection bias. let average_estimate = (adjusted_estimate + linear_estimate) / 2.0; @@ -232,45 +323,6 @@ impl Estimator { linear_estimate } } - - pub fn hip_accum(&self) -> f64 { - match self.estimate_state { - EstimateState::Hip(hip_accum) => hip_accum, - EstimateState::Composite => 0.0, - } - } - - pub fn kxq0(&self) -> f64 { - self.kxq0 - } - - pub fn kxq1(&self) -> f64 { - self.kxq1 - } - - pub fn uses_composite_estimate(&self) -> bool { - matches!(self.estimate_state, EstimateState::Composite) - } - - pub fn estimate_state(&self) -> EstimateState { - self.estimate_state - } - - /// Restores estimate state after copying or transforming the same logical sketch. - pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimate_state = state; - } - - /// Invalidates HIP after registers from independent histories are merged. - pub fn invalidate_hip(&mut self) { - self.estimate_state = EstimateState::Composite; - } - - /// Replaces register-derived KxQ values after a bulk register operation. - pub fn restore_kxq(&mut self, kxq0: f64, kxq1: f64) { - self.kxq0 = kxq0; - self.kxq1 = kxq1; - } } /// Relative error used as `estimate / (1 + error)`: negative for upper bounds. @@ -448,6 +500,33 @@ mod tests { use super::*; + #[test] + fn restored_hip_supports_updates_after_deferred_reconstruction() { + let lg_config_k = 4; + let mut registers = [0; 16]; + let mut eager = Estimator::new(lg_config_k); + for (slot, value) in [(0, 4), (1, 32), (2, 63)] { + eager.update(lg_config_k, registers[slot], value); + registers[slot] = value; + } + let history = eager.estimate_state(); + let mut deferred = eager.clone(); + deferred.invalidate(); + deferred.restore_estimate_state(history, registers.iter().copied()); + + for (slot, value) in [(3, 2), (0, 33), (3, 5)] { + eager.update(lg_config_k, registers[slot], value); + deferred.update(lg_config_k, registers[slot], value); + registers[slot] = value; + } + + assert_eq!(deferred.estimate_state(), eager.estimate_state()); + assert_eq!( + deferred.statistics(registers.iter().copied()), + eager.statistics(registers.iter().copied()) + ); + } + #[test] fn lower_bound_is_clamped_to_the_non_zero_register_count() { let lg_config_k = 4; @@ -457,7 +536,7 @@ mod tests { } assert_eq!( - estimator.lower_bound(lg_config_k, 0, 8, NumStdDev::Three), + estimator.lower_bound(lg_config_k, std::iter::empty(), NumStdDev::Three), 8.0 ); } @@ -471,7 +550,7 @@ mod tests { } assert_eq!( - estimator.lower_bound(lg_config_k, 1, 16, NumStdDev::Three), + estimator.lower_bound(lg_config_k, std::iter::empty(), NumStdDev::Three), 16.0 ); } @@ -481,8 +560,8 @@ mod tests { let est = Estimator::new(10); // 1024 registers assert_eq!(est.hip_accum(), 0.0); - assert_eq!(est.kxq0(), 1024.0); // All zeros = 1.0 each - assert_eq!(est.kxq1(), 0.0); + assert_eq!(est.statistics(std::iter::empty()).kxq0, 1024.0); // All zeros = 1.0 each + assert_eq!(est.statistics(std::iter::empty()).kxq1, 0.0); assert!(!est.uses_composite_estimate()); } @@ -494,8 +573,8 @@ mod tests { assert_that!(est.hip_accum(), gt(0.0)); - assert_that!(est.kxq0(), lt(256.0)); - assert_eq!(est.kxq1(), 0.0); + assert_that!(est.statistics(std::iter::empty()).kxq0, lt(256.0)); + assert_eq!(est.statistics(std::iter::empty()).kxq1, 0.0); } #[test] @@ -503,37 +582,37 @@ mod tests { let mut est = Estimator::new(8); est.update(8, 0, 10); - let kxq0_after_10 = est.kxq0(); - let kxq1_after_10 = est.kxq1(); + let kxq0_after_10 = est.statistics(std::iter::empty()).kxq0; + let kxq1_after_10 = est.statistics(std::iter::empty()).kxq1; assert_that!(kxq0_after_10, lt(256.0)); assert_eq!(kxq1_after_10, 0.0); // Update from 10 to 50 (crosses the 32 boundary) est.update(8, 10, 50); - let kxq0_after_50 = est.kxq0(); - let kxq1_after_50 = est.kxq1(); + let kxq0_after_50 = est.statistics(std::iter::empty()).kxq0; + let kxq1_after_50 = est.statistics(std::iter::empty()).kxq1; assert_that!(kxq0_after_50, lt(kxq0_after_10)); // Removed 1/2^10 from kxq0 (decreases kxq0) assert_that!(kxq1_after_50, gt(0.0)); // Added 1/2^50 to kxq1 } #[test] - fn test_invalidate_hip() { + fn test_restore_composite_estimate() { let mut est = Estimator::new(10); est.update(10, 0, 5); let hip_normal = est.hip_accum(); assert_that!(hip_normal, gt(0.0)); - est.invalidate_hip(); + est.restore_estimate_state(EstimateState::Composite, std::iter::empty()); assert!(est.uses_composite_estimate()); assert_eq!(est.hip_accum(), 0.0); // Register-derived state continues to update after HIP is invalidated. - let kxq0_before = est.kxq0(); + let kxq0_before = est.statistics(std::iter::empty()).kxq0; est.update(10, 5, 10); assert_eq!(est.hip_accum(), 0.0); - assert_ne!(est.kxq0(), kxq0_before); + assert_ne!(est.statistics(std::iter::empty()).kxq0, kxq0_before); } } diff --git a/tests-integration/tests/hll_test/estimate.rs b/tests-integration/tests/hll_test/estimate.rs index 37242598..ef0f5e8d 100644 --- a/tests-integration/tests/hll_test/estimate.rs +++ b/tests-integration/tests/hll_test/estimate.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use datasketches::common::NumStdDev; use datasketches::hll::HllSketch; use datasketches::hll::HllType; use datasketches::hll::HllUnion; @@ -23,6 +24,44 @@ use googletest::prelude::near; const HLL_TYPES: [HllType; 3] = [HllType::Hll4, HllType::Hll6, HllType::Hll8]; +#[test] +fn estimates_survive_minimum_shifts_and_round_trips() { + let mut sketches = HLL_TYPES.map(|hll_type| HllSketch::new(4, hll_type).unwrap()); + let mut start = 0; + // At this precision, the growing stream fills every register and repeatedly raises + // HLL4's packing minimum. Its minimum count must not be treated as a zero count. + for end in [64, 512, 4_096] { + for value in start..end { + for sketch in &mut sketches { + sketch.update(value as u64); + } + } + let expected = &sketches[2]; + for sketch in &sketches { + assert_eq!(sketch.estimate(), expected.estimate()); + assert_eq!(sketch.composite_estimate(), expected.composite_estimate()); + for num_std_dev in [NumStdDev::One, NumStdDev::Two, NumStdDev::Three] { + assert_eq!( + sketch.lower_bound(num_std_dev), + expected.lower_bound(num_std_dev) + ); + assert_eq!( + sketch.upper_bound(num_std_dev), + expected.upper_bound(num_std_dev) + ); + } + } + for sketch in &mut sketches { + *sketch = HllSketch::deserialize(&sketch.serialize()).unwrap(); + } + start = end; + } + assert_eq!(sketches[0].estimate(), sketches[2].estimate()); + assert_eq!( + sketches[0].composite_estimate(), + sketches[2].composite_estimate() + ); +} fn make_sketch(hll_type: HllType, values: impl Iterator) -> HllSketch { let mut sketch = HllSketch::new(11, hll_type).unwrap(); for value in values { From 0b2e63846390fa7a70a5103241dd8954f27e3868 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 11:17:43 +0800 Subject: [PATCH 6/9] refactor(hll): bind estimation state to owned registers Keep dense registers and their estimation state in a single HllArray owner. Queries resolve statistics from that owner's registers, and bulk mutations invalidate statistics before writing, including when an input iterator panics partway through a merge. Select statistics storage by register representation. HLL4 and HLL6 maintain KxQ eagerly and use their existing zero-count metadata; only HLL8 uses OnceLock for deferred reconstruction. This restores the 88-byte HllSketch layout on aarch64 without changing the serialized representation. Route dense coupon batches directly to their stable register representation instead of dispatching through the sketch mode for each coupon. Cover HIP restoration, occupied-register bounds, and interrupted bulk writes at the owning-array boundary. --- datasketches/src/hll/array.rs | 333 ++++++++++++++++++++++++++++ datasketches/src/hll/array4.rs | 295 +++++++++++-------------- datasketches/src/hll/array6.rs | 166 ++++++-------- datasketches/src/hll/array8.rs | 300 ++++++++++--------------- datasketches/src/hll/estimator.rs | 356 +++++------------------------- datasketches/src/hll/mod.rs | 1 + datasketches/src/hll/sketch.rs | 6 +- datasketches/src/hll/union.rs | 33 ++- 8 files changed, 722 insertions(+), 768 deletions(-) create mode 100644 datasketches/src/hll/array.rs diff --git a/datasketches/src/hll/array.rs b/datasketches/src/hll/array.rs new file mode 100644 index 00000000..b4a2ad4d --- /dev/null +++ b/datasketches/src/hll/array.rs @@ -0,0 +1,333 @@ +// 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. + +use std::fmt::Debug; +use std::sync::OnceLock; + +use crate::common::NumStdDev; +use crate::hll::Coupon; +use crate::hll::array8::Registers8; +use crate::hll::estimator::EstimateState; +use crate::hll::estimator::Kxq; +use crate::hll::estimator::RegisterStatistics; +use crate::hll::estimator::relative_error; + +/// A dense sketch's registers and the estimation state derived from those same registers. +/// +/// All register mutations pass through this owner, so a caller cannot query statistics for +/// a different register array or forget to invalidate statistics after a bulk merge. +#[derive(Debug, Clone)] +pub struct HllArray { + registers: R, + state: EstimateState, + statistics: R::Statistics, +} + +/// Physical register storage and the statistics maintenance it supports. +pub trait Registers: Debug + Clone + PartialEq { + type Statistics: StatisticsStorage; + fn new(lg_config_k: u8) -> Self; + fn lg_config_k(&self) -> u8; + fn get(&self, slot: u32) -> u8; + fn values(&self) -> impl Iterator; + fn num_zeros(&self) -> u32; + fn is_empty(&self) -> bool; + fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)>; +} + +/// Eager sums for packed storage, or deferred statistics for bulk-mergeable storage. +pub trait StatisticsStorage: Debug + Clone { + fn from_serialized(kxq: Kxq, num_zeros: u32) -> Self; + fn resolve(&self, registers: &impl Registers) -> RegisterStatistics; + fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8); + fn cached_num_zeros(&self) -> Option { + None + } +} + +impl StatisticsStorage for Kxq { + fn from_serialized(kxq: Kxq, _: u32) -> Self { + kxq + } + + fn resolve(&self, registers: &impl Registers) -> RegisterStatistics { + RegisterStatistics { + num_zeros: registers.num_zeros(), + kxq: *self, + } + } + + #[inline] + fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8) { + state.update(lg_config_k, self, old, new); + } +} + +impl StatisticsStorage for OnceLock { + fn from_serialized(kxq: Kxq, num_zeros: u32) -> Self { + OnceLock::from(RegisterStatistics { num_zeros, kxq }) + } + + fn resolve(&self, registers: &impl Registers) -> RegisterStatistics { + *self.get_or_init(|| RegisterStatistics::from_registers(registers.values())) + } + + #[inline] + fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8) { + if let Some(statistics) = self.get_mut() { + state.update(lg_config_k, &mut statistics.kxq, old, new); + statistics.num_zeros -= u32::from(old == 0); + } else { + debug_assert_eq!(*state, EstimateState::Composite); + } + } + + fn cached_num_zeros(&self) -> Option { + self.get().map(|statistics| statistics.num_zeros) + } +} + +impl PartialEq for HllArray { + fn eq(&self, other: &Self) -> bool { + self.registers == other.registers + && self.state == other.state + && self.statistics() == other.statistics() + } +} + +impl HllArray { + pub fn new(lg_config_k: u8) -> Self { + Self::from_serialized( + R::new(lg_config_k), + 0.0, + (1 << lg_config_k) as f64, + 0.0, + 1 << lg_config_k, + false, + ) + } + + pub fn from_serialized( + registers: R, + hip_accum: f64, + kxq0: f64, + kxq1: f64, + num_zeros: u32, + out_of_order: bool, + ) -> Self { + Self { + registers, + state: EstimateState::from_serialized(hip_accum, out_of_order), + statistics: R::Statistics::from_serialized( + Kxq { + low: kxq0, + high: kxq1, + }, + num_zeros, + ), + } + } + + /// Returns storage for read-only encoding and representation-specific inspection. + pub fn registers(&self) -> &R { + &self.registers + } + + pub fn lg_config_k(&self) -> u8 { + self.registers.lg_config_k() + } + + pub fn num_registers(&self) -> usize { + 1 << self.lg_config_k() + } + + pub fn get(&self, slot: u32) -> u8 { + self.registers.get(slot) + } + + pub fn estimate_state(&self) -> EstimateState { + self.state + } + + pub fn statistics(&self) -> RegisterStatistics { + self.statistics.resolve(&self.registers) + } + + #[inline] + pub fn update(&mut self, coupon: Coupon) { + if let Some((old, new)) = self.registers.update(coupon) { + self.statistics + .update(&mut self.state, self.registers.lg_config_k(), old, new); + } + } + + pub fn restore_estimate_state(&mut self, state: EstimateState) { + if let EstimateState::Hip(_) = state { + // Resuming HIP requires current sums before the next scalar update. + self.statistics(); + } + self.state = state; + } + + pub fn estimate(&self) -> f64 { + match self.state { + EstimateState::Hip(value) => value, + EstimateState::Composite => self.composite_estimate(), + } + } + + pub fn composite_estimate(&self) -> f64 { + self.statistics().composite_estimate(self.lg_config_k()) + } + + pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { + let rse = relative_error( + self.lg_config_k(), + true, + self.state.uses_composite_estimate(), + num_std_dev, + ); + self.estimate() / (1.0 + rse) + } + + pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { + let statistics = self.statistics(); + let estimate = match self.state { + EstimateState::Hip(value) => value, + EstimateState::Composite => statistics.composite_estimate(self.lg_config_k()), + }; + let rse = relative_error( + self.lg_config_k(), + false, + self.state.uses_composite_estimate(), + num_std_dev, + ); + (estimate / (1.0 + rse)).max(f64::from((1 << self.lg_config_k()) - statistics.num_zeros)) + } + + pub fn is_empty(&self) -> bool { + match self.statistics.cached_num_zeros() { + Some(num_zeros) => num_zeros == (1 << self.lg_config_k()), + // Do not rebuild statistics just to answer emptiness between bulk merges. + None => self.registers.is_empty(), + } + } +} + +impl HllArray { + pub fn values(&self) -> &[u8] { + self.registers.as_slice() + } + + fn invalidate(&mut self) { + self.state = EstimateState::Composite; + self.statistics.take(); + } + + pub fn merge_registers(&mut self, values: impl Iterator) { + // Invalidate before consuming the iterator so a panic cannot leave stale statistics. + self.invalidate(); + self.registers.merge_registers(values); + } + + pub fn merge_array_same_lgk(&mut self, src: &[u8]) { + assert_eq!( + src.len(), + self.num_registers(), + "Source and destination must have same lg_k" + ); + self.invalidate(); + self.registers.merge_same_lgk(src); + } + + pub fn merge_array_with_downsample(&mut self, src: &[u8], src_lg_k: u8) { + assert!( + src_lg_k > self.lg_config_k(), + "Source lg_k must be greater than destination lg_k for downsampling" + ); + assert_eq!( + src.len(), + 1 << src_lg_k, + "Source length must match 2^src_lg_k" + ); + self.invalidate(); + self.registers.merge_with_downsample(src); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::hll::array4::Registers4; + use crate::hll::array6::Registers6; + use crate::hll::array8::Array8; + + #[test] + fn lower_bounds_respect_occupied_registers_in_every_representation() { + fn check() { + let mut array = HllArray::::new(4); + for slot in 0..8 { + array.update(Coupon::pack(slot, 1)); + } + assert_eq!(array.lower_bound(NumStdDev::Three), 8.0); + for slot in 8..16 { + array.update(Coupon::pack(slot, 1)); + } + assert_eq!(array.lower_bound(NumStdDev::Three), 16.0); + } + check::(); + check::(); + check::(); + } + + #[test] + fn restored_hip_supports_updates_after_deferred_reconstruction() { + let mut eager = Array8::new(4); + for (slot, value) in [(0, 4), (1, 32), (2, 63)] { + eager.update(Coupon::pack(slot, value)); + } + let history = eager.estimate_state(); + let mut deferred = eager.clone(); + // Merging zero registers leaves the data unchanged but discards update history. + deferred.merge_array_same_lgk(&[0; 16]); + deferred.restore_estimate_state(history); + for (slot, value) in [(3, 2), (0, 33), (3, 5)] { + eager.update(Coupon::pack(slot, value)); + deferred.update(Coupon::pack(slot, value)); + } + assert_eq!(deferred.estimate(), eager.estimate()); + assert_eq!(deferred.serialize(), eager.serialize()); + } + + #[test] + fn interrupted_bulk_merge_cannot_leave_stale_statistics() { + let mut array = Array8::new(4); + array.update(Coupon::pack(0, 1)); + let mut expected = array.clone(); + expected.merge_registers(std::iter::once((1, 5))); + + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + array.merge_registers((1..3).map(|slot| { + assert_eq!(slot, 1, "interrupted register input"); + (slot, 5) + })); + })); + assert!(result.is_err()); + assert_eq!(array.estimate(), expected.estimate()); + assert_eq!(array.serialize(), expected.serialize()); + } +} diff --git a/datasketches/src/hll/array4.rs b/datasketches/src/hll/array4.rs index 309d0e4c..3e7650fd 100644 --- a/datasketches/src/hll/array4.rs +++ b/datasketches/src/hll/array4.rs @@ -19,12 +19,12 @@ use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; -use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; +use crate::hll::array::HllArray; +use crate::hll::array::Registers; use crate::hll::aux_map::AuxMap; -use crate::hll::estimator::EstimateState; -use crate::hll::estimator::Estimator; +use crate::hll::estimator::Kxq; use crate::hll::serialization::COMPACT_FLAG_MASK; use crate::hll::serialization::COUPON_SIZE_BYTES; use crate::hll::serialization::CUR_MODE_HLL; @@ -55,7 +55,7 @@ impl AuxFormat { /// Registers stored as four-bit offsets from their current minimum. #[derive(Debug, Clone, PartialEq)] -pub struct Array4 { +pub struct Registers4 { lg_config_k: u8, /// Even slots occupy the low nibble; odd slots occupy the high nibble. bytes: Box<[u8]>, @@ -64,23 +64,11 @@ pub struct Array4 { num_at_cur_min: u32, /// Every `AUX_TOKEN` nibble has an entry here containing the absolute register value. aux_map: Option, - estimator: Estimator, } -impl Array4 { - pub fn new(lg_config_k: u8) -> Self { - let num_bytes = 1 << (lg_config_k - 1); - let num_at_cur_min = 1 << lg_config_k; - Self { - lg_config_k, - bytes: vec![0u8; num_bytes].into_boxed_slice(), - cur_min: 0, - num_at_cur_min, - aux_map: None, - estimator: Estimator::new(lg_config_k), - } - } +pub type Array4 = HllArray; +impl Registers4 { #[inline] fn get_raw(&self, slot: u32) -> u8 { debug_assert!(slot >> 1 < self.bytes.len() as u32); @@ -89,18 +77,6 @@ impl Array4 { if slot & 1 == 0 { byte & 15 } else { byte >> 4 } } - pub fn get(&self, slot: u32) -> u8 { - read_register(&self.bytes, self.cur_min, self.aux_map.as_ref(), slot) - } - - pub fn num_registers(&self) -> usize { - 1 << self.lg_config_k - } - - pub fn estimate_state(&self) -> EstimateState { - self.estimator.estimate_state() - } - #[inline] fn put_raw(&mut self, slot: u32, value: u8) { debug_assert!(value <= AUX_TOKEN); @@ -115,72 +91,6 @@ impl Array4 { }; } - pub fn update(&mut self, coupon: Coupon) { - let mask = (1 << self.lg_config_k) - 1; - let slot = coupon.slot() & mask; - let new_value = coupon.value(); - - if new_value <= self.cur_min { - return; - } - - let raw_stored = self.get_raw(slot); - let lower_bound = raw_stored + self.cur_min; - - if new_value <= lower_bound { - return; - } - - let old_value = if raw_stored < AUX_TOKEN { - lower_bound - } else { - self.aux_map - .as_ref() - .expect("aux_map should be initialized since stored value is AUX_TOKEN") - .get(slot) - .expect("slot should be in aux_map since associated value is AUX_TOKEN") - }; - - if new_value <= old_value { - return; - } - - self.estimator - .update(self.lg_config_k, old_value, new_value); - - let shifted_new = new_value - self.cur_min; - - match (raw_stored, shifted_new) { - (AUX_TOKEN, shifted) if shifted >= AUX_TOKEN => { - self.aux_map - .as_mut() - .expect("aux_map should be initialized since stored value is AUX_TOKEN") - .replace(slot, new_value); - } - // Registers only increase here; an exception cannot disappear until cur_min changes. - (AUX_TOKEN, _) => { - unreachable!("AUX_TOKEN present with non-exception new value"); - } - (_, shifted) if shifted >= AUX_TOKEN => { - self.put_raw(slot, AUX_TOKEN); - let aux = self - .aux_map - .get_or_insert_with(|| AuxMap::new(self.lg_config_k)); - aux.insert(slot, new_value); - } - _ => { - self.put_raw(slot, shifted_new); - } - } - - if old_value == self.cur_min { - self.num_at_cur_min -= 1; - while self.num_at_cur_min == 0 { - self.shift_to_bigger_cur_min(); - } - } - } - /// Requires no registers at `cur_min`. Raising the offset makes some auxiliary values /// fit in the nibbles again; the logical register values must stay unchanged. fn shift_to_bigger_cur_min(&mut self) { @@ -226,45 +136,113 @@ impl Array4 { self.cur_min = new_cur_min; self.num_at_cur_min = num_at_new; } +} - fn register_values(&self) -> impl Iterator { - (0..self.num_registers() as u32).map(|slot| self.get(slot)) +impl Registers for Registers4 { + type Statistics = Kxq; + fn new(lg_config_k: u8) -> Self { + let num_bytes = 1 << (lg_config_k - 1); + let num_at_cur_min = 1 << lg_config_k; + Self { + lg_config_k, + bytes: vec![0u8; num_bytes].into_boxed_slice(), + cur_min: 0, + num_at_cur_min, + aux_map: None, + } } - pub fn estimate(&self) -> f64 { - self.estimator - .estimate(self.lg_config_k, self.register_values()) + fn lg_config_k(&self) -> u8 { + self.lg_config_k } - pub fn composite_estimate(&self) -> f64 { - self.estimator - .composite_estimate(self.lg_config_k, self.register_values()) + fn get(&self, slot: u32) -> u8 { + read_register(&self.bytes, self.cur_min, self.aux_map.as_ref(), slot) } - pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .upper_bound(self.lg_config_k, self.register_values(), num_std_dev) + fn values(&self) -> impl Iterator { + (0..1 << self.lg_config_k).map(|slot| self.get(slot)) } - pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .lower_bound(self.lg_config_k, self.register_values(), num_std_dev) + fn num_zeros(&self) -> u32 { + if self.cur_min == 0 { + self.num_at_cur_min + } else { + 0 + } } - pub fn restore_estimate_state(&mut self, state: EstimateState) { - let bytes = &self.bytes; - let cur_min = self.cur_min; - let aux_map = self.aux_map.as_ref(); - let registers = - (0..1 << self.lg_config_k).map(|slot| read_register(bytes, cur_min, aux_map, slot)); - self.estimator.restore_estimate_state(state, registers); + fn is_empty(&self) -> bool { + self.num_zeros() == (1 << self.lg_config_k) } - pub fn is_empty(&self) -> bool { - self.estimator - .is_empty(self.lg_config_k, self.register_values()) + #[inline] + fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { + let mask = (1 << self.lg_config_k) - 1; + let slot = coupon.slot() & mask; + let new_value = coupon.value(); + + if new_value <= self.cur_min { + return None; + } + + let raw_stored = self.get_raw(slot); + let lower_bound = raw_stored + self.cur_min; + + if new_value <= lower_bound { + return None; + } + + let old_value = if raw_stored < AUX_TOKEN { + lower_bound + } else { + self.aux_map + .as_ref() + .expect("aux_map should be initialized since stored value is AUX_TOKEN") + .get(slot) + .expect("slot should be in aux_map since associated value is AUX_TOKEN") + }; + + if new_value <= old_value { + return None; + } + + let shifted_new = new_value - self.cur_min; + + match (raw_stored, shifted_new) { + (AUX_TOKEN, shifted) if shifted >= AUX_TOKEN => { + self.aux_map + .as_mut() + .expect("aux_map should be initialized since stored value is AUX_TOKEN") + .replace(slot, new_value); + } + // Registers only increase here; an exception cannot disappear until cur_min changes. + (AUX_TOKEN, _) => { + unreachable!("AUX_TOKEN present with non-exception new value"); + } + (_, shifted) if shifted >= AUX_TOKEN => { + self.put_raw(slot, AUX_TOKEN); + let aux = self + .aux_map + .get_or_insert_with(|| AuxMap::new(self.lg_config_k)); + aux.insert(slot, new_value); + } + _ => { + self.put_raw(slot, shifted_new); + } + } + + if old_value == self.cur_min { + self.num_at_cur_min -= 1; + while self.num_at_cur_min == 0 { + self.shift_to_bigger_cur_min(); + } + } + Some((old_value, new_value)) } +} +impl Array4 { pub fn deserialize( mut cursor: SketchSlice, cur_min: u8, @@ -353,30 +331,30 @@ impl Array4 { aux_map = Some(aux); } - let estimator = Estimator::from_serialized( + Ok(Self::from_serialized( + Registers4 { + lg_config_k, + bytes: data.into_boxed_slice(), + cur_min, + num_at_cur_min, + aux_map, + }, hip_accum, kxq0, kxq1, if cur_min == 0 { num_at_cur_min } else { 0 }, ooo, - ); - - Ok(Self { - lg_config_k, - bytes: data.into_boxed_slice(), - cur_min, - num_at_cur_min, - aux_map, - estimator, - }) + )) } /// Writes packed registers followed by compact auxiliary entries. - pub fn serialize(&self, lg_config_k: u8) -> Vec { - let statistics = self.estimator.statistics(self.register_values()); + pub fn serialize(&self) -> Vec { + let registers = self.registers(); + let lg_config_k = self.lg_config_k(); + let statistics = self.statistics(); let num_bytes = 1 << (lg_config_k - 1); - let aux_entries: Vec<(u32, u8)> = if let Some(aux) = &self.aux_map { + let aux_entries: Vec<(u32, u8)> = if let Some(aux) = ®isters.aux_map { aux.iter().collect() } else { vec![] @@ -393,24 +371,24 @@ impl Array4 { bytes.write_u8(0); // lg_arr is unused for compact auxiliary storage. let mut flags = COMPACT_FLAG_MASK; - if self.estimator.uses_composite_estimate() { + if self.estimate_state().uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); - bytes.write_u8(self.cur_min); + bytes.write_u8(registers.cur_min); bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL4)); - bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(statistics.kxq0); - bytes.write_f64_le(statistics.kxq1); + bytes.write_f64_le(self.estimate_state().hip_accum()); + bytes.write_f64_le(statistics.kxq.low); + bytes.write_f64_le(statistics.kxq.high); - bytes.write_u32_le(self.num_at_cur_min); + bytes.write_u32_le(registers.num_at_cur_min); bytes.write_u32_le(aux_count); - bytes.write(&self.bytes); + bytes.write(®isters.bytes); for (slot, value) in aux_entries.iter().copied() { bytes.write_u32_le(Coupon::pack(slot, value).raw()); @@ -421,8 +399,9 @@ impl Array4 { /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.bytes.len() + self.registers().bytes.len() + self + .registers() .aux_map .as_ref() .map(|a| a.estimated_size()) @@ -454,7 +433,7 @@ mod tests { #[test] fn test_get_set_raw() { - let mut data = Array4::new(4); // 16 buckets + let mut data = Registers4::new(4); // 16 buckets // Test even slot (low nibble) data.put_raw(0, 5); @@ -502,20 +481,11 @@ mod tests { // kxq0 should have decreased (we removed a 0 and added a 10) // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq0, - lt(256.0) - ); + assert_that!(arr.statistics().kxq.low, lt(256.0)); // kxq1 should have a small positive value (from 1/2^40) - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq1, - gt(0.0) - ); - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq1, - lt(0.001) - ); + assert_that!(arr.statistics().kxq.high, gt(0.0)); + assert_that!(arr.statistics().kxq.high, lt(0.001)); } #[test] @@ -525,18 +495,21 @@ mod tests { let mut arr = Array4::new(lg_config_k); arr.update(Coupon::pack(0, 15)); - assert_eq!(arr.get_raw(0), AUX_TOKEN); - assert_eq!(arr.aux_map.as_ref().and_then(|aux| aux.get(0)), Some(15)); + assert_eq!(arr.registers().get_raw(0), AUX_TOKEN); + assert_eq!( + arr.registers().aux_map.as_ref().and_then(|aux| aux.get(0)), + Some(15) + ); for slot in 1..num_slots { arr.update(Coupon::pack(slot, 1)); } - assert_eq!(arr.cur_min, 1); - assert_eq!(arr.num_at_cur_min, num_slots - 1); - assert_eq!(arr.get_raw(0), 14); + assert_eq!(arr.registers().cur_min, 1); + assert_eq!(arr.registers().num_at_cur_min, num_slots - 1); + assert_eq!(arr.registers().get_raw(0), 14); assert_eq!(arr.get(0), 15); - assert_that!(arr.aux_map, none()); + assert_that!(arr.registers().aux_map, none()); for slot in 1..num_slots { assert_eq!(arr.get(slot), 1); diff --git a/datasketches/src/hll/array6.rs b/datasketches/src/hll/array6.rs index 74b89d39..05143158 100644 --- a/datasketches/src/hll/array6.rs +++ b/datasketches/src/hll/array6.rs @@ -19,11 +19,11 @@ use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; -use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; -use crate::hll::estimator::EstimateState; -use crate::hll::estimator::Estimator; +use crate::hll::array::HllArray; +use crate::hll::array::Registers; +use crate::hll::estimator::Kxq; use crate::hll::serialization::CUR_MODE_HLL; use crate::hll::serialization::HLL_PREAMBLE_SIZE; use crate::hll::serialization::HLL_PREINTS; @@ -36,42 +36,20 @@ const VAL_MASK_6: u16 = 0x3F; /// Registers packed into consecutive six-bit fields. #[derive(Debug, Clone, PartialEq)] -pub struct Array6 { +pub struct Registers6 { lg_config_k: u8, bytes: Box<[u8]>, - estimator: Estimator, + num_zeros: u32, } -impl Array6 { - pub fn new(lg_config_k: u8) -> Self { - let k = 1 << lg_config_k; - let num_bytes = num_bytes_for_k(k); - - Self { - lg_config_k, - bytes: vec![0u8; num_bytes].into_boxed_slice(), - estimator: Estimator::new(lg_config_k), - } - } +pub type Array6 = HllArray; +impl Registers6 { #[inline] fn get_raw(&self, slot: u32) -> u8 { read_register(&self.bytes, slot) } - #[inline] - pub fn get(&self, slot: u32) -> u8 { - self.get_raw(slot) - } - - pub fn num_registers(&self) -> usize { - 1 << self.lg_config_k - } - - pub fn estimate_state(&self) -> EstimateState { - self.estimator.estimate_state() - } - #[inline] fn put_raw(&mut self, slot: u32, value: u8) { debug_assert!(value <= 63, "6-bit value must be 0-63"); @@ -90,58 +68,62 @@ impl Array6 { self.bytes[byte_idx] = bytes_out[0]; self.bytes[byte_idx + 1] = bytes_out[1]; } +} - #[inline] - pub fn update(&mut self, coupon: Coupon) { - let mask = (1 << self.lg_config_k) - 1; - let slot = coupon.slot() & mask; - let new_value = coupon.value(); - - let old_value = self.get_raw(slot); - - if new_value > old_value { - self.estimator - .update(self.lg_config_k, old_value, new_value); +impl Registers for Registers6 { + type Statistics = Kxq; + fn new(lg_config_k: u8) -> Self { + let k = 1 << lg_config_k; + let num_bytes = num_bytes_for_k(k); - self.put_raw(slot, new_value); + Self { + lg_config_k, + bytes: vec![0u8; num_bytes].into_boxed_slice(), + num_zeros: k, } } - fn register_values(&self) -> impl Iterator { - (0..self.num_registers() as u32).map(|slot| self.get(slot)) + fn lg_config_k(&self) -> u8 { + self.lg_config_k } - pub fn estimate(&self) -> f64 { - self.estimator - .estimate(self.lg_config_k, self.register_values()) + fn get(&self, slot: u32) -> u8 { + self.get_raw(slot) } - pub fn composite_estimate(&self) -> f64 { - self.estimator - .composite_estimate(self.lg_config_k, self.register_values()) + fn values(&self) -> impl Iterator { + (0..1 << self.lg_config_k).map(|slot| self.get(slot)) } - pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .upper_bound(self.lg_config_k, self.register_values(), num_std_dev) + fn num_zeros(&self) -> u32 { + self.num_zeros } - pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .lower_bound(self.lg_config_k, self.register_values(), num_std_dev) + fn is_empty(&self) -> bool { + self.num_zeros() == (1 << self.lg_config_k) } - pub fn restore_estimate_state(&mut self, state: EstimateState) { - let bytes = &self.bytes; - let registers = (0..1 << self.lg_config_k).map(|slot| read_register(bytes, slot)); - self.estimator.restore_estimate_state(state, registers); - } + #[inline] + fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { + let mask = (1 << self.lg_config_k) - 1; + let slot = coupon.slot() & mask; + let new_value = coupon.value(); + + let old_value = self.get_raw(slot); - pub fn is_empty(&self) -> bool { - self.estimator - .is_empty(self.lg_config_k, self.register_values()) + if new_value > old_value { + self.put_raw(slot, new_value); + if old_value == 0 { + self.num_zeros -= 1; + } + Some((old_value, new_value)) + } else { + None + } } +} +impl Array6 { pub fn deserialize_registers( mut cursor: SketchSlice, lg_config_k: u8, @@ -181,17 +163,24 @@ impl Array6 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, num_zeros, ooo); - - Ok(Self { - lg_config_k, - bytes: data.into_boxed_slice(), - estimator, - }) + Ok(Self::from_serialized( + Registers6 { + lg_config_k, + bytes: data.into_boxed_slice(), + num_zeros, + }, + hip_accum, + kxq0, + kxq1, + num_zeros, + ooo, + )) } - pub fn serialize(&self, lg_config_k: u8) -> Vec { - let statistics = self.estimator.statistics(self.register_values()); + pub fn serialize(&self) -> Vec { + let registers = self.registers(); + let lg_config_k = self.lg_config_k(); + let statistics = self.statistics(); let k = 1 << lg_config_k; let num_bytes = num_bytes_for_k(k); let total_size = HLL_PREAMBLE_SIZE + num_bytes; @@ -204,7 +193,7 @@ impl Array6 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if self.estimator.uses_composite_estimate() { + if self.estimate_state().uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -214,9 +203,9 @@ impl Array6 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL6)); - bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(statistics.kxq0); - bytes.write_f64_le(statistics.kxq1); + bytes.write_f64_le(self.estimate_state().hip_accum()); + bytes.write_f64_le(statistics.kxq.low); + bytes.write_f64_le(statistics.kxq.high); // Write num_at_cur_min (num_zeros for Array6) bytes.write_u32_le(statistics.num_zeros); @@ -224,14 +213,14 @@ impl Array6 { // Write aux_count (always 0 for Array6) bytes.write_u32_le(0); - bytes.write(&self.bytes); + bytes.write(®isters.bytes); bytes.into_bytes() } /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.bytes.len() + self.registers().bytes.len() } } @@ -269,7 +258,7 @@ mod tests { #[test] fn test_get_set_raw() { - let mut arr = Array6::new(4); // 16 slots + let mut arr = Registers6::new(4); // 16 slots arr.put_raw(0, 0); arr.put_raw(1, 1); @@ -296,7 +285,7 @@ mod tests { #[test] fn test_boundary_crossing() { - let mut arr = Array6::new(8); // 256 slots + let mut arr = Registers6::new(8); // 256 slots // Slot 1: starts at bit 6 (crosses byte 0/1 boundary) arr.put_raw(1, 0b111111); @@ -336,7 +325,7 @@ mod tests { #[test] fn test_full_range() { - let mut arr = Array6::new(6); // 64 slots + let mut arr = Registers6::new(6); // 64 slots for val in 0..64u8 { arr.put_raw(val as u32, val); @@ -355,19 +344,10 @@ mod tests { arr.update(Coupon::pack(1, 40)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq0, - lt(256.0) - ); + assert_that!(arr.statistics().kxq.low, lt(256.0)); // kxq1 should have a small positive value (from 1/2^40) - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq1, - gt(0.0) - ); - assert_that!( - arr.estimator.statistics(arr.register_values()).kxq1, - lt(0.001) - ); + assert_that!(arr.statistics().kxq.high, gt(0.0)); + assert_that!(arr.statistics().kxq.high, lt(0.001)); } } diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index 04037a65..693e5831 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -15,15 +15,17 @@ // specific language governing permissions and limitations // under the License. +use std::sync::OnceLock; + use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; -use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; -use crate::hll::estimator::EstimateState; -use crate::hll::estimator::Estimator; +use crate::hll::array::HllArray; +use crate::hll::array::Registers; +use crate::hll::estimator::RegisterStatistics; use crate::hll::serialization::CUR_MODE_HLL; use crate::hll::serialization::HLL_PREAMBLE_SIZE; use crate::hll::serialization::HLL_PREINTS; @@ -33,149 +35,84 @@ use crate::hll::serialization::TGT_HLL8; use crate::hll::serialization::encode_mode_byte; /// Registers stored one per byte. -#[derive(Debug, Clone)] -pub struct Array8 { +#[derive(Debug, Clone, PartialEq)] +pub struct Registers8 { lg_config_k: u8, bytes: Box<[u8]>, - estimator: Estimator, } -impl PartialEq for Array8 { - fn eq(&self, other: &Self) -> bool { - self.lg_config_k == other.lg_config_k - && self.bytes == other.bytes - && self.estimator.estimate_state() == other.estimator.estimate_state() - && self.estimator.statistics(self.bytes.iter().copied()) - == other.estimator.statistics(other.bytes.iter().copied()) - } -} - -impl Array8 { - pub fn new(lg_config_k: u8) -> Self { - let k = 1 << lg_config_k; +pub type Array8 = HllArray; +impl Registers for Registers8 { + type Statistics = OnceLock; + fn new(lg_config_k: u8) -> Self { Self { lg_config_k, - bytes: vec![0u8; k as usize].into_boxed_slice(), - estimator: Estimator::new(lg_config_k), + bytes: vec![0; 1 << lg_config_k].into_boxed_slice(), } } - #[inline] - pub fn get(&self, slot: u32) -> u8 { - self.bytes[slot as usize] - } - - #[inline] - fn put(&mut self, slot: u32, value: u8) { - self.bytes[slot as usize] = value; + fn lg_config_k(&self) -> u8 { + self.lg_config_k } - #[inline] - pub fn update(&mut self, coupon: Coupon) { - let mask = (1 << self.lg_config_k) - 1; - let slot = coupon.slot() & mask; - let new_value = coupon.value(); - - let old_value = self.get(slot); - - if new_value > old_value { - self.estimator - .update(self.lg_config_k, old_value, new_value); - self.put(slot, new_value); - } - } - - pub fn estimate(&self) -> f64 { - self.estimator - .estimate(self.lg_config_k, self.bytes.iter().copied()) + fn get(&self, slot: u32) -> u8 { + self.bytes[slot as usize] } - pub fn composite_estimate(&self) -> f64 { - self.estimator - .composite_estimate(self.lg_config_k, self.bytes.iter().copied()) + fn values(&self) -> impl Iterator { + self.bytes.iter().copied() } - pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .upper_bound(self.lg_config_k, self.bytes.iter().copied(), num_std_dev) + fn num_zeros(&self) -> u32 { + self.bytes.iter().filter(|&&value| value == 0).count() as u32 } - pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - self.estimator - .lower_bound(self.lg_config_k, self.bytes.iter().copied(), num_std_dev) + fn is_empty(&self) -> bool { + self.bytes.iter().all(|&value| value == 0) } - pub fn is_empty(&self) -> bool { - self.estimator - .is_empty(self.lg_config_k, self.bytes.iter().copied()) + #[inline] + fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { + let slot = (coupon.slot() & ((1 << self.lg_config_k) - 1)) as usize; + let old = self.bytes[slot]; + let new = coupon.value(); + if new > old { + self.bytes[slot] = new; + Some((old, new)) + } else { + None + } } +} - pub fn values(&self) -> &[u8] { +impl Registers8 { + pub fn as_slice(&self) -> &[u8] { &self.bytes } - pub fn num_registers(&self) -> usize { - 1 << self.lg_config_k - } - - pub fn estimate_state(&self) -> EstimateState { - self.estimator.estimate_state() - } - - pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimator - .restore_estimate_state(state, self.bytes.iter().copied()); - } - - /// Bypasses estimator updates. Call `invalidate_estimator` after all writes. - pub fn set_register(&mut self, slot: usize, value: u8) { - self.bytes[slot] = value; - } - - /// Invalidates HIP and defers rebuilding register-derived fields until a query needs them. - pub fn invalidate_estimator(&mut self) { - self.estimator.invalidate(); + pub fn merge_registers(&mut self, values: impl Iterator) { + for (slot, value) in values { + self.bytes[slot] = self.bytes[slot].max(value); + } } - /// Merges registers and invalidates HIP because their update order is unavailable. - pub fn merge_array_same_lgk(&mut self, src: &[u8]) { - assert_eq!( - src.len(), - self.bytes.len(), - "Source and destination must have same lg_k" - ); - - for (i, &val) in src.iter().enumerate() { - self.bytes[i] = self.bytes[i].max(val); + pub fn merge_same_lgk(&mut self, values: &[u8]) { + for (dst, &src) in self.bytes.iter_mut().zip(values) { + *dst = (*dst).max(src); } - - self.invalidate_estimator(); } - /// Register values come from a separate hash word, so folding slot indices needs no - /// adjustment to the values. Merging independent histories invalidates HIP. - pub fn merge_array_with_downsample(&mut self, src: &[u8], src_lg_k: u8) { - assert!( - src_lg_k > self.lg_config_k, - "Source lg_k must be greater than destination lg_k for downsampling" - ); - assert_eq!( - src.len(), - 1 << src_lg_k, - "Source length must match 2^src_lg_k" - ); - - let dst_mask = (1 << self.lg_config_k) - 1; - - for (src_slot, &val) in src.iter().enumerate() { - let dst_slot = (src_slot as u32 & dst_mask) as usize; - self.bytes[dst_slot] = self.bytes[dst_slot].max(val); + pub fn merge_with_downsample(&mut self, values: &[u8]) { + let mask = (1 << self.lg_config_k) - 1; + for (src_slot, &value) in values.iter().enumerate() { + let dst_slot = (src_slot as u32 & mask) as usize; + self.bytes[dst_slot] = self.bytes[dst_slot].max(value); } - - self.invalidate_estimator(); } +} +impl Array8 { pub fn deserialize_registers( mut cursor: SketchSlice, lg_config_k: u8, @@ -214,17 +151,22 @@ impl Array8 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, num_zeros, ooo); - - Ok(Self { - lg_config_k, - bytes: data.into_boxed_slice(), - estimator, - }) - } - - pub fn serialize(&self, lg_config_k: u8) -> Vec { - let statistics = self.estimator.statistics(self.bytes.iter().copied()); + Ok(Self::from_serialized( + Registers8 { + lg_config_k, + bytes: data.into_boxed_slice(), + }, + hip_accum, + kxq0, + kxq1, + num_zeros, + ooo, + )) + } + + pub fn serialize(&self) -> Vec { + let lg_config_k = self.lg_config_k(); + let statistics = self.statistics(); let k = 1 << lg_config_k; let total_size = HLL_PREAMBLE_SIZE + k as usize; let mut bytes = SketchBytes::with_capacity(total_size); @@ -236,7 +178,7 @@ impl Array8 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if self.estimator.uses_composite_estimate() { + if self.estimate_state().uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -246,9 +188,9 @@ impl Array8 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL8)); - bytes.write_f64_le(self.estimator.hip_accum()); - bytes.write_f64_le(statistics.kxq0); - bytes.write_f64_le(statistics.kxq1); + bytes.write_f64_le(self.estimate_state().hip_accum()); + bytes.write_f64_le(statistics.kxq.low); + bytes.write_f64_le(statistics.kxq.high); // Write num_at_cur_min (num_zeros for Array8) bytes.write_u32_le(statistics.num_zeros); @@ -256,14 +198,13 @@ impl Array8 { // Write aux_count (always 0 for Array8) bytes.write_u32_le(0); - bytes.write(&self.bytes); + bytes.write(self.values()); bytes.into_bytes() } - /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.bytes.len() + self.values().len() } } @@ -288,19 +229,19 @@ mod tests { #[test] fn test_get_set() { - let mut arr = Array8::new(4); // 16 slots + let mut arr = Registers8::new(4); // 16 slots for slot in 0..16 { - arr.put(slot, (slot * 17) as u8); + arr.bytes[slot] = (slot * 17) as u8; } for slot in 0..16 { assert_eq!(arr.get(slot), (slot * 17) as u8); } - arr.put(0, 0); - arr.put(1, 127); - arr.put(2, 255); + arr.bytes[0] = 0; + arr.bytes[1] = 127; + arr.bytes[2] = 255; assert_eq!(arr.get(0), 0); assert_eq!(arr.get(1), 127); @@ -345,10 +286,10 @@ mod tests { #[test] fn test_full_value_range() { - let mut arr = Array8::new(8); // 256 slots + let mut arr = Registers8::new(8); // 256 slots for val in 0..=255u8 { - arr.put(val as u32, val); + arr.bytes[val as usize] = val; } for val in 0..=255u8 { @@ -358,13 +299,13 @@ mod tests { #[test] fn test_high_value_direct() { - let mut arr = Array8::new(6); // 64 slots + let mut arr = Registers8::new(6); // 64 slots // Direct storage accepts byte values beyond the six-bit coupon range. let test_values = [16, 32, 64, 128, 200, 255]; for (slot, &value) in test_values.iter().enumerate() { - arr.put(slot as u32, value); + arr.bytes[slot] = value; assert_eq!(arr.get(slot as u32), value); } @@ -381,29 +322,20 @@ mod tests { arr.update(Coupon::pack(1, 50)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!( - arr.estimator.statistics(arr.bytes.iter().copied()).kxq0, - lt(256.0) - ); + assert_that!(arr.statistics().kxq.low, lt(256.0)); // kxq1 should have a positive value (from 1/2^50) - assert_that!( - arr.estimator.statistics(arr.bytes.iter().copied()).kxq1, - gt(0.0) - ); - assert_that!( - arr.estimator.statistics(arr.bytes.iter().copied()).kxq1, - lt(1e-10) - ); + assert_that!(arr.statistics().kxq.high, gt(0.0)); + assert_that!(arr.statistics().kxq.high, lt(1e-10)); } #[test] fn test_values_access() { let mut arr = Array8::new(4); // 16 slots - arr.put(0, 10); - arr.put(5, 25); - arr.put(15, 63); + arr.merge_registers(std::iter::once((0, 10))); + arr.merge_registers(std::iter::once((5, 25))); + arr.merge_registers(std::iter::once((15, 63))); let vals = arr.values(); assert_eq!(vals.len(), 16); @@ -418,13 +350,13 @@ mod tests { let mut dst = Array8::new(4); // 16 slots let mut src = Array8::new(4); // 16 slots - dst.put(0, 10); - dst.put(1, 20); - dst.put(2, 30); + dst.merge_registers(std::iter::once((0, 10))); + dst.merge_registers(std::iter::once((1, 20))); + dst.merge_registers(std::iter::once((2, 30))); - src.put(1, 15); // Smaller than dst[1]=20, should keep 20 - src.put(2, 35); // Larger than dst[2]=30, should update to 35 - src.put(3, 40); // New value + src.merge_registers(std::iter::once((1, 15))); // Smaller than dst[1]=20, should keep 20 + src.merge_registers(std::iter::once((2, 35))); // Larger than dst[2]=30, should update to 35 + src.merge_registers(std::iter::once((3, 40))); // New value dst.merge_array_same_lgk(src.values()); @@ -434,14 +366,9 @@ mod tests { assert_eq!(dst.get(3), 40, "dst[3] got new value"); // Bulk merges require composite estimation. - assert!(dst.estimator.uses_composite_estimate()); + assert!(dst.estimate_state().uses_composite_estimate()); - assert_eq!( - dst.estimator - .statistics(dst.bytes.iter().copied()) - .num_zeros, - 12 - ); + assert_eq!(dst.statistics().num_zeros, 12); } #[test] @@ -450,14 +377,14 @@ mod tests { let mut dst = Array8::new(4); // 16 slots let mut src = Array8::new(5); // 32 slots - dst.put(0, 10); - dst.put(1, 20); + dst.merge_registers(std::iter::once((0, 10))); + dst.merge_registers(std::iter::once((1, 20))); // Set up src - slots 0 and 16 both map to dst slot 0 - src.put(0, 15); // maps to dst[0], max(10, 15) = 15 - src.put(16, 25); // maps to dst[0], max(15, 25) = 25 - src.put(1, 18); // maps to dst[1], max(20, 18) = 20 - src.put(17, 30); // maps to dst[1], max(20, 30) = 30 + src.merge_registers(std::iter::once((0, 15))); // maps to dst[0], max(10, 15) = 15 + src.merge_registers(std::iter::once((16, 25))); // maps to dst[0], max(15, 25) = 25 + src.merge_registers(std::iter::once((1, 18))); // maps to dst[1], max(20, 18) = 20 + src.merge_registers(std::iter::once((17, 30))); // maps to dst[1], max(20, 30) = 30 dst.merge_array_with_downsample(src.values(), 5); @@ -465,7 +392,7 @@ mod tests { assert_eq!(dst.get(1), 30, "dst[1] = max(20, 18, 30)"); // Bulk merges require composite estimation. - assert!(dst.estimator.uses_composite_estimate()); + assert!(dst.estimate_state().uses_composite_estimate()); } #[test] @@ -490,22 +417,21 @@ mod tests { fn test_rebuild_cached_values() { let mut arr = Array8::new(4); // 16 slots - arr.put(0, 10); - arr.put(1, 20); - arr.put(2, 30); + arr.merge_registers(std::iter::once((0, 10))); + arr.merge_registers(std::iter::once((1, 20))); + arr.merge_registers(std::iter::once((2, 30))); - arr.put(3, 31); - arr.put(4, 32); - arr.put(5, 63); - arr.invalidate_estimator(); + arr.merge_registers(std::iter::once((3, 31))); + arr.merge_registers(std::iter::once((4, 32))); + arr.merge_registers(std::iter::once((5, 63))); - let statistics = arr.estimator.statistics(arr.bytes.iter().copied()); + let statistics = arr.statistics(); assert_eq!(statistics.num_zeros, 10); assert_eq!( - statistics.kxq0, + statistics.kxq.low, 10.0 + 2_f64.powi(-10) + 2_f64.powi(-20) + 2_f64.powi(-30) + 2_f64.powi(-31) ); - assert_eq!(statistics.kxq1, 2_f64.powi(-32) + 2_f64.powi(-63)); + assert_eq!(statistics.kxq.high, 2_f64.powi(-32) + 2_f64.powi(-63)); } #[test] @@ -514,11 +440,11 @@ mod tests { let mut src = Array8::new(4); for i in 0..16 { - dst.put(i, i as u8); + dst.merge_registers(std::iter::once((i as usize, i as u8))); } for i in 0..16 { - src.put(i, (15 - i) as u8); + src.merge_registers(std::iter::once((i as usize, (15 - i) as u8))); } dst.merge_array_same_lgk(src.values()); diff --git a/datasketches/src/hll/estimator.rs b/datasketches/src/hll/estimator.rs index 0141dd97..9b672ba4 100644 --- a/datasketches/src/hll/estimator.rs +++ b/datasketches/src/hll/estimator.rs @@ -15,16 +15,11 @@ // specific language governing permissions and limitations // under the License. -//! Cardinality estimation for HLL-mode sketches. -//! -//! Sequential register updates use HIP (Historical Inverse Probability). Bulk merges use the -//! composite estimator because register values do not retain their update order. +//! Cardinality estimation from HLL register statistics and update history. //! //! Estimator formulas and crossover constants follow datasketches-cpp: //! -use std::sync::OnceLock; - use crate::common::NumStdDev; use crate::common::inv_pow2::inv_pow2; use crate::hll::composite_interpolation; @@ -40,210 +35,88 @@ pub enum EstimateState { Composite, } -/// Estimation history and lazily materialized statistics of the logical registers. -/// -/// HIP depends on update order and cannot be reconstructed from register values. Bulk merges -/// discard that history and invalidate only the derived statistics. Queries share a single -/// reconstruction until the next bulk merge; scalar updates maintain materialized statistics. -#[derive(Debug, Clone, PartialEq)] -pub struct Estimator { - state: EstimateState, - statistics: OnceLock, -} - -/// Representation-independent statistics derived entirely from the current register values. -/// -/// Splitting the sum of `2^-register` at value 32 preserves precision when large and small -/// contributions coexist. HLL4's packing minimum and its count are separate storage metadata. -#[derive(Debug, Clone, Copy, PartialEq)] -pub struct RegisterStatistics { - pub num_zeros: u32, - pub kxq0: f64, - pub kxq1: f64, -} - -impl Estimator { - pub fn new(lg_config_k: u8) -> Self { - let k = 1 << lg_config_k; - Self { - state: EstimateState::Hip(0.0), - statistics: OnceLock::from(RegisterStatistics { - num_zeros: k, - kxq0: k as f64, - kxq1: 0.0, - }), - } - } - - /// Restores history and statistics from an HLL serialization preamble. - pub fn from_serialized( - hip_accum: f64, - kxq0: f64, - kxq1: f64, - num_zeros: u32, - out_of_order: bool, - ) -> Self { - Self { - state: if out_of_order { - EstimateState::Composite - } else { - EstimateState::Hip(hip_accum) - }, - statistics: OnceLock::from(RegisterStatistics { - num_zeros, - kxq0, - kxq1, - }), - } - } - - /// Applies a register increase; requires `new_value > old_value`. - #[inline] - pub fn update(&mut self, lg_config_k: u8, old_value: u8, new_value: u8) { - if let Some(statistics) = self.statistics.get_mut() { - // HIP must use the sums from before this register update. - if let EstimateState::Hip(hip_accum) = &mut self.state { - *hip_accum += (1 << lg_config_k) as f64 / (statistics.kxq0 + statistics.kxq1); - } - statistics.update(old_value, new_value); +impl EstimateState { + pub fn from_serialized(hip_accum: f64, out_of_order: bool) -> Self { + if out_of_order { + Self::Composite } else { - debug_assert_eq!(self.state, EstimateState::Composite); + Self::Hip(hip_accum) } } - pub fn statistics(&self, registers: impl Iterator) -> &RegisterStatistics { - self.statistics - .get_or_init(|| RegisterStatistics::from_registers(registers)) - } - - pub fn estimate(&self, lg_config_k: u8, registers: impl Iterator) -> f64 { - match self.state { - EstimateState::Hip(hip_accum) => hip_accum, - EstimateState::Composite => self.composite_estimate(lg_config_k, registers), + pub fn hip_accum(self) -> f64 { + match self { + Self::Hip(value) => value, + Self::Composite => 0.0, } } - pub fn composite_estimate(&self, lg_config_k: u8, registers: impl Iterator) -> f64 { - self.statistics(registers).composite_estimate(lg_config_k) + pub fn uses_composite_estimate(self) -> bool { + matches!(self, Self::Composite) } - pub fn upper_bound( - &self, - lg_config_k: u8, - registers: impl Iterator, - num_std_dev: NumStdDev, - ) -> f64 { - let estimate = self.estimate(lg_config_k, registers); - let rse = relative_error( - lg_config_k, - true, - self.uses_composite_estimate(), - num_std_dev, - ); - estimate / (1.0 + rse) + #[inline] + pub fn update(&mut self, lg_config_k: u8, kxq: &mut Kxq, old_value: u8, new_value: u8) { + // HIP uses the sums from before this register increase. + if let Self::Hip(hip_accum) = self { + *hip_accum += (1 << lg_config_k) as f64 / (kxq.low + kxq.high); + } + kxq.update(old_value, new_value); } +} - pub fn lower_bound( - &self, - lg_config_k: u8, - registers: impl Iterator, - num_std_dev: NumStdDev, - ) -> f64 { - let statistics = self.statistics(registers); - let estimate = match self.state { - EstimateState::Hip(hip_accum) => hip_accum, - EstimateState::Composite => statistics.composite_estimate(lg_config_k), - }; - let rse = relative_error( - lg_config_k, - false, - self.uses_composite_estimate(), - num_std_dev, - ); - // Each nonzero register requires at least one distinct item. - (estimate / (1.0 + rse)).max(f64::from((1 << lg_config_k) - statistics.num_zeros)) - } +/// Sums of `2^-register`, split at register value 32 to preserve precision. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct Kxq { + pub low: f64, + pub high: f64, +} - pub fn is_empty(&self, lg_config_k: u8, mut registers: impl Iterator) -> bool { - if let Some(statistics) = self.statistics.get() { - statistics.num_zeros == (1 << lg_config_k) +impl Kxq { + #[inline] + fn update(&mut self, old_value: u8, new_value: u8) { + if old_value < 32 { + self.low -= inv_pow2(old_value); } else { - // Union checks emptiness between merges; stop at the first nonzero register - // without materializing statistics that the next merge would invalidate. - registers.all(|value| value == 0) - } - } - - pub fn hip_accum(&self) -> f64 { - match self.state { - EstimateState::Hip(hip_accum) => hip_accum, - EstimateState::Composite => 0.0, + self.high -= inv_pow2(old_value); } - } - - pub fn uses_composite_estimate(&self) -> bool { - matches!(self.state, EstimateState::Composite) - } - - pub fn estimate_state(&self) -> EstimateState { - self.state - } - - /// Restores history after copying or transforming the same logical sketch. - pub fn restore_estimate_state( - &mut self, - state: EstimateState, - registers: impl Iterator, - ) { - if let EstimateState::Hip(_) = state { - // Subsequent scalar updates need the sums to advance the restored HIP accumulator. - self.statistics(registers); + if new_value < 32 { + self.low += inv_pow2(new_value); + } else { + self.high += inv_pow2(new_value); } - self.state = state; } +} - /// Discards update history and defers rebuilding statistics after a bulk register write. - pub fn invalidate(&mut self) { - self.state = EstimateState::Composite; - self.statistics.take(); - } +/// Representation-independent inputs to composite estimation. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct RegisterStatistics { + pub num_zeros: u32, + pub kxq: Kxq, } impl RegisterStatistics { - fn from_registers(registers: impl Iterator) -> Self { + pub fn from_registers(registers: impl Iterator) -> Self { let mut statistics = Self { num_zeros: 0, - kxq0: 0.0, - kxq1: 0.0, + kxq: Kxq { + low: 0.0, + high: 0.0, + }, }; for value in registers { if value == 0 { statistics.num_zeros += 1; - statistics.kxq0 += 1.0; + statistics.kxq.low += 1.0; } else if value < 32 { - statistics.kxq0 += 1.0 / (1u64 << value) as f64; + statistics.kxq.low += 1.0 / (1u64 << value) as f64; } else { - statistics.kxq1 += 1.0 / (1u64 << value) as f64; + statistics.kxq.high += 1.0 / (1u64 << value) as f64; } } statistics } - #[inline] - fn update(&mut self, old_value: u8, new_value: u8) { - self.num_zeros -= u32::from(old_value == 0); - if old_value < 32 { - self.kxq0 -= inv_pow2(old_value); - } else { - self.kxq1 -= inv_pow2(old_value); - } - if new_value < 32 { - self.kxq0 += inv_pow2(new_value); - } else { - self.kxq1 += inv_pow2(new_value); - } - } - fn raw_estimate(&self, lg_config_k: u8) -> f64 { let k = (1 << lg_config_k) as f64; @@ -255,7 +128,7 @@ impl RegisterStatistics { _ => 0.7213 / (1.0 + 1.079 / k), }; - (correction_factor * k * k) / (self.kxq0 + self.kxq1) + (correction_factor * k * k) / (self.kxq.low + self.kxq.high) } fn bitmap_estimate(&self, lg_config_k: u8) -> f64 { @@ -328,7 +201,7 @@ impl RegisterStatistics { /// Relative error used as `estimate / (1 + error)`: negative for upper bounds. /// Follows datasketches-cpp's `HllUtil::getRelErr`: /// -fn relative_error( +pub fn relative_error( lg_config_k: u8, upper_bound: bool, composite: bool, @@ -491,128 +364,3 @@ static NON_HIP_UB: [f64; 27] = [ -0.031827816, -0.046973459, // lg_k=12 ]; - -#[cfg(test)] -mod tests { - use googletest::assert_that; - use googletest::prelude::gt; - use googletest::prelude::lt; - - use super::*; - - #[test] - fn restored_hip_supports_updates_after_deferred_reconstruction() { - let lg_config_k = 4; - let mut registers = [0; 16]; - let mut eager = Estimator::new(lg_config_k); - for (slot, value) in [(0, 4), (1, 32), (2, 63)] { - eager.update(lg_config_k, registers[slot], value); - registers[slot] = value; - } - let history = eager.estimate_state(); - let mut deferred = eager.clone(); - deferred.invalidate(); - deferred.restore_estimate_state(history, registers.iter().copied()); - - for (slot, value) in [(3, 2), (0, 33), (3, 5)] { - eager.update(lg_config_k, registers[slot], value); - deferred.update(lg_config_k, registers[slot], value); - registers[slot] = value; - } - - assert_eq!(deferred.estimate_state(), eager.estimate_state()); - assert_eq!( - deferred.statistics(registers.iter().copied()), - eager.statistics(registers.iter().copied()) - ); - } - - #[test] - fn lower_bound_is_clamped_to_the_non_zero_register_count() { - let lg_config_k = 4; - let mut estimator = Estimator::new(lg_config_k); - for _ in 0..8 { - estimator.update(lg_config_k, 0, 1); - } - - assert_eq!( - estimator.lower_bound(lg_config_k, std::iter::empty(), NumStdDev::Three), - 8.0 - ); - } - - #[test] - fn lower_bound_is_clamped_to_config_k_when_every_register_is_hit() { - let lg_config_k = 4; - let mut estimator = Estimator::new(lg_config_k); - for _ in 0..16 { - estimator.update(lg_config_k, 0, 1); - } - - assert_eq!( - estimator.lower_bound(lg_config_k, std::iter::empty(), NumStdDev::Three), - 16.0 - ); - } - - #[test] - fn test_estimator_initialization() { - let est = Estimator::new(10); // 1024 registers - - assert_eq!(est.hip_accum(), 0.0); - assert_eq!(est.statistics(std::iter::empty()).kxq0, 1024.0); // All zeros = 1.0 each - assert_eq!(est.statistics(std::iter::empty()).kxq1, 0.0); - assert!(!est.uses_composite_estimate()); - } - - #[test] - fn test_estimator_update() { - let mut est = Estimator::new(8); // 256 registers - - est.update(8, 0, 10); - - assert_that!(est.hip_accum(), gt(0.0)); - - assert_that!(est.statistics(std::iter::empty()).kxq0, lt(256.0)); - assert_eq!(est.statistics(std::iter::empty()).kxq1, 0.0); - } - - #[test] - fn test_kxq_split() { - let mut est = Estimator::new(8); - - est.update(8, 0, 10); - let kxq0_after_10 = est.statistics(std::iter::empty()).kxq0; - let kxq1_after_10 = est.statistics(std::iter::empty()).kxq1; - - assert_that!(kxq0_after_10, lt(256.0)); - assert_eq!(kxq1_after_10, 0.0); - - // Update from 10 to 50 (crosses the 32 boundary) - est.update(8, 10, 50); - let kxq0_after_50 = est.statistics(std::iter::empty()).kxq0; - let kxq1_after_50 = est.statistics(std::iter::empty()).kxq1; - - assert_that!(kxq0_after_50, lt(kxq0_after_10)); // Removed 1/2^10 from kxq0 (decreases kxq0) - assert_that!(kxq1_after_50, gt(0.0)); // Added 1/2^50 to kxq1 - } - - #[test] - fn test_restore_composite_estimate() { - let mut est = Estimator::new(10); - - est.update(10, 0, 5); - let hip_normal = est.hip_accum(); - assert_that!(hip_normal, gt(0.0)); - - est.restore_estimate_state(EstimateState::Composite, std::iter::empty()); - assert!(est.uses_composite_estimate()); - assert_eq!(est.hip_accum(), 0.0); - - // Register-derived state continues to update after HIP is invalidated. - let kxq0_before = est.statistics(std::iter::empty()).kxq0; - est.update(10, 5, 10); - assert_eq!(est.hip_accum(), 0.0); - assert_ne!(est.statistics(std::iter::empty()).kxq0, kxq0_before); - } -} diff --git a/datasketches/src/hll/mod.rs b/datasketches/src/hll/mod.rs index 06db8d35..0c08fa4d 100644 --- a/datasketches/src/hll/mod.rs +++ b/datasketches/src/hll/mod.rs @@ -61,6 +61,7 @@ use std::hash::Hash; use crate::hash::MurmurHash3X64128; +mod array; mod array4; mod array6; mod array8; diff --git a/datasketches/src/hll/sketch.rs b/datasketches/src/hll/sketch.rs index 910a730c..70282e17 100644 --- a/datasketches/src/hll/sketch.rs +++ b/datasketches/src/hll/sketch.rs @@ -464,9 +464,9 @@ impl HllSketch { match &self.mode { Mode::List { list, hll_type } => list.serialize(self.lg_config_k, *hll_type), Mode::Set { set, hll_type } => set.serialize(self.lg_config_k, *hll_type), - Mode::Array4(arr) => arr.serialize(self.lg_config_k), - Mode::Array6(arr) => arr.serialize(self.lg_config_k), - Mode::Array8(arr) => arr.serialize(self.lg_config_k), + Mode::Array4(arr) => arr.serialize(), + Mode::Array6(arr) => arr.serialize(), + Mode::Array8(arr) => arr.serialize(), } } diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index cef8bf9f..a4e38e65 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -415,6 +415,12 @@ fn convert_coupon_mode_to_hll8(src_mode: &Mode, src_lg_k: u8) -> HllSketch { } fn merge_coupons_into_gadget(gadget: &mut HllSketch, src_mode: &Mode) { + // Dense coupon updates cannot change representation, so dispatch once for the batch. + if let Mode::Array8(dst) = gadget.mode_mut() { + merge_coupons_into_mode(dst, src_mode); + return; + } + match src_mode { Mode::List { list, .. } => { for coupon in list.container().iter() { @@ -484,14 +490,7 @@ fn get_array_estimate_state(mode: &Mode) -> EstimateState { } fn merge_array46_same_lgk(dst: &mut Array8, num_registers: usize, get_value: impl Fn(u32) -> u8) { - for slot in 0..num_registers { - let val = get_value(slot as u32); - let current = dst.values()[slot]; - if val > current { - dst.set_register(slot, val); - } - } - dst.invalidate_estimator(); + dst.merge_registers((0..num_registers).map(|slot| (slot, get_value(slot as u32)))); } /// Merging independent register histories invalidates the HIP accumulator. @@ -518,18 +517,12 @@ fn merge_array46_with_downsample( num_registers: usize, get_value: impl Fn(u32) -> u8, ) { - let dst_mask = (1 << dst_lg_k) - 1; - for src_slot in 0..num_registers { - let val = get_value(src_slot as u32); - if val > 0 { - let dst_slot = (src_slot as u32 & dst_mask) as usize; - let current = dst.values()[dst_slot]; - if val > current { - dst.set_register(dst_slot, val); - } - } - } - dst.invalidate_estimator(); + let mask = (1 << dst_lg_k) - 1; + dst.merge_registers( + (0..num_registers) + .map(|slot| (slot & mask, get_value(slot as u32))) + .filter(|&(_, value)| value > 0), + ); } /// Multiple source registers map to each destination register via masking. From 66976bfba48923a4f0732c5b94dae5b6d16a2c5e Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 11:18:04 +0800 Subject: [PATCH 7/9] revert: keep deferred estimation local to HLL8 Revert the shared estimator and owning-array refactors to keep the deferred reconstruction implementation specific to Array8. Packed representations continue to maintain their estimator fields directly without a common statistics-storage abstraction. Retain direct HLL8 result serialization, fast adoption of dense HLL8 inputs, and deferred reconstruction across dense merges. This reverts commits 0b2e638 and 6eb6363. The resulting tree matches fb2c360. --- benchmarks/hll/update.rs | 6 +- datasketches/src/hll/array.rs | 333 ------------------ datasketches/src/hll/array4.rs | 309 +++++++++-------- datasketches/src/hll/array6.rs | 173 +++++----- datasketches/src/hll/array8.rs | 346 +++++++++++++------ datasketches/src/hll/estimator.rs | 307 ++++++++++++---- datasketches/src/hll/mod.rs | 1 - datasketches/src/hll/sketch.rs | 6 +- datasketches/src/hll/union.rs | 33 +- tests-integration/tests/hll_test/estimate.rs | 39 --- 10 files changed, 750 insertions(+), 803 deletions(-) delete mode 100644 datasketches/src/hll/array.rs diff --git a/benchmarks/hll/update.rs b/benchmarks/hll/update.rs index ca38bcc0..88a335c1 100644 --- a/benchmarks/hll/update.rs +++ b/benchmarks/hll/update.rs @@ -25,10 +25,10 @@ use divan::counter::ItemsCount; use crate::hash_inputs::ITEMS; use crate::hash_inputs::bytes_32_values; -#[divan::bench(args = [HllType::Hll4, HllType::Hll6, HllType::Hll8])] -fn u64(bencher: Bencher, hll_type: HllType) { +#[divan::bench] +fn u64(bencher: Bencher) { bencher.counter(ItemsCount::new(ITEMS)).bench_local(|| { - let mut sketch = HllSketch::new(12, hll_type).unwrap(); + let mut sketch = HllSketch::new(12, HllType::Hll8).unwrap(); for value in 0..ITEMS as u64 { sketch.update(black_box(value)); } diff --git a/datasketches/src/hll/array.rs b/datasketches/src/hll/array.rs deleted file mode 100644 index b4a2ad4d..00000000 --- a/datasketches/src/hll/array.rs +++ /dev/null @@ -1,333 +0,0 @@ -// 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. - -use std::fmt::Debug; -use std::sync::OnceLock; - -use crate::common::NumStdDev; -use crate::hll::Coupon; -use crate::hll::array8::Registers8; -use crate::hll::estimator::EstimateState; -use crate::hll::estimator::Kxq; -use crate::hll::estimator::RegisterStatistics; -use crate::hll::estimator::relative_error; - -/// A dense sketch's registers and the estimation state derived from those same registers. -/// -/// All register mutations pass through this owner, so a caller cannot query statistics for -/// a different register array or forget to invalidate statistics after a bulk merge. -#[derive(Debug, Clone)] -pub struct HllArray { - registers: R, - state: EstimateState, - statistics: R::Statistics, -} - -/// Physical register storage and the statistics maintenance it supports. -pub trait Registers: Debug + Clone + PartialEq { - type Statistics: StatisticsStorage; - fn new(lg_config_k: u8) -> Self; - fn lg_config_k(&self) -> u8; - fn get(&self, slot: u32) -> u8; - fn values(&self) -> impl Iterator; - fn num_zeros(&self) -> u32; - fn is_empty(&self) -> bool; - fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)>; -} - -/// Eager sums for packed storage, or deferred statistics for bulk-mergeable storage. -pub trait StatisticsStorage: Debug + Clone { - fn from_serialized(kxq: Kxq, num_zeros: u32) -> Self; - fn resolve(&self, registers: &impl Registers) -> RegisterStatistics; - fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8); - fn cached_num_zeros(&self) -> Option { - None - } -} - -impl StatisticsStorage for Kxq { - fn from_serialized(kxq: Kxq, _: u32) -> Self { - kxq - } - - fn resolve(&self, registers: &impl Registers) -> RegisterStatistics { - RegisterStatistics { - num_zeros: registers.num_zeros(), - kxq: *self, - } - } - - #[inline] - fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8) { - state.update(lg_config_k, self, old, new); - } -} - -impl StatisticsStorage for OnceLock { - fn from_serialized(kxq: Kxq, num_zeros: u32) -> Self { - OnceLock::from(RegisterStatistics { num_zeros, kxq }) - } - - fn resolve(&self, registers: &impl Registers) -> RegisterStatistics { - *self.get_or_init(|| RegisterStatistics::from_registers(registers.values())) - } - - #[inline] - fn update(&mut self, state: &mut EstimateState, lg_config_k: u8, old: u8, new: u8) { - if let Some(statistics) = self.get_mut() { - state.update(lg_config_k, &mut statistics.kxq, old, new); - statistics.num_zeros -= u32::from(old == 0); - } else { - debug_assert_eq!(*state, EstimateState::Composite); - } - } - - fn cached_num_zeros(&self) -> Option { - self.get().map(|statistics| statistics.num_zeros) - } -} - -impl PartialEq for HllArray { - fn eq(&self, other: &Self) -> bool { - self.registers == other.registers - && self.state == other.state - && self.statistics() == other.statistics() - } -} - -impl HllArray { - pub fn new(lg_config_k: u8) -> Self { - Self::from_serialized( - R::new(lg_config_k), - 0.0, - (1 << lg_config_k) as f64, - 0.0, - 1 << lg_config_k, - false, - ) - } - - pub fn from_serialized( - registers: R, - hip_accum: f64, - kxq0: f64, - kxq1: f64, - num_zeros: u32, - out_of_order: bool, - ) -> Self { - Self { - registers, - state: EstimateState::from_serialized(hip_accum, out_of_order), - statistics: R::Statistics::from_serialized( - Kxq { - low: kxq0, - high: kxq1, - }, - num_zeros, - ), - } - } - - /// Returns storage for read-only encoding and representation-specific inspection. - pub fn registers(&self) -> &R { - &self.registers - } - - pub fn lg_config_k(&self) -> u8 { - self.registers.lg_config_k() - } - - pub fn num_registers(&self) -> usize { - 1 << self.lg_config_k() - } - - pub fn get(&self, slot: u32) -> u8 { - self.registers.get(slot) - } - - pub fn estimate_state(&self) -> EstimateState { - self.state - } - - pub fn statistics(&self) -> RegisterStatistics { - self.statistics.resolve(&self.registers) - } - - #[inline] - pub fn update(&mut self, coupon: Coupon) { - if let Some((old, new)) = self.registers.update(coupon) { - self.statistics - .update(&mut self.state, self.registers.lg_config_k(), old, new); - } - } - - pub fn restore_estimate_state(&mut self, state: EstimateState) { - if let EstimateState::Hip(_) = state { - // Resuming HIP requires current sums before the next scalar update. - self.statistics(); - } - self.state = state; - } - - pub fn estimate(&self) -> f64 { - match self.state { - EstimateState::Hip(value) => value, - EstimateState::Composite => self.composite_estimate(), - } - } - - pub fn composite_estimate(&self) -> f64 { - self.statistics().composite_estimate(self.lg_config_k()) - } - - pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - let rse = relative_error( - self.lg_config_k(), - true, - self.state.uses_composite_estimate(), - num_std_dev, - ); - self.estimate() / (1.0 + rse) - } - - pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - let statistics = self.statistics(); - let estimate = match self.state { - EstimateState::Hip(value) => value, - EstimateState::Composite => statistics.composite_estimate(self.lg_config_k()), - }; - let rse = relative_error( - self.lg_config_k(), - false, - self.state.uses_composite_estimate(), - num_std_dev, - ); - (estimate / (1.0 + rse)).max(f64::from((1 << self.lg_config_k()) - statistics.num_zeros)) - } - - pub fn is_empty(&self) -> bool { - match self.statistics.cached_num_zeros() { - Some(num_zeros) => num_zeros == (1 << self.lg_config_k()), - // Do not rebuild statistics just to answer emptiness between bulk merges. - None => self.registers.is_empty(), - } - } -} - -impl HllArray { - pub fn values(&self) -> &[u8] { - self.registers.as_slice() - } - - fn invalidate(&mut self) { - self.state = EstimateState::Composite; - self.statistics.take(); - } - - pub fn merge_registers(&mut self, values: impl Iterator) { - // Invalidate before consuming the iterator so a panic cannot leave stale statistics. - self.invalidate(); - self.registers.merge_registers(values); - } - - pub fn merge_array_same_lgk(&mut self, src: &[u8]) { - assert_eq!( - src.len(), - self.num_registers(), - "Source and destination must have same lg_k" - ); - self.invalidate(); - self.registers.merge_same_lgk(src); - } - - pub fn merge_array_with_downsample(&mut self, src: &[u8], src_lg_k: u8) { - assert!( - src_lg_k > self.lg_config_k(), - "Source lg_k must be greater than destination lg_k for downsampling" - ); - assert_eq!( - src.len(), - 1 << src_lg_k, - "Source length must match 2^src_lg_k" - ); - self.invalidate(); - self.registers.merge_with_downsample(src); - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::hll::array4::Registers4; - use crate::hll::array6::Registers6; - use crate::hll::array8::Array8; - - #[test] - fn lower_bounds_respect_occupied_registers_in_every_representation() { - fn check() { - let mut array = HllArray::::new(4); - for slot in 0..8 { - array.update(Coupon::pack(slot, 1)); - } - assert_eq!(array.lower_bound(NumStdDev::Three), 8.0); - for slot in 8..16 { - array.update(Coupon::pack(slot, 1)); - } - assert_eq!(array.lower_bound(NumStdDev::Three), 16.0); - } - check::(); - check::(); - check::(); - } - - #[test] - fn restored_hip_supports_updates_after_deferred_reconstruction() { - let mut eager = Array8::new(4); - for (slot, value) in [(0, 4), (1, 32), (2, 63)] { - eager.update(Coupon::pack(slot, value)); - } - let history = eager.estimate_state(); - let mut deferred = eager.clone(); - // Merging zero registers leaves the data unchanged but discards update history. - deferred.merge_array_same_lgk(&[0; 16]); - deferred.restore_estimate_state(history); - for (slot, value) in [(3, 2), (0, 33), (3, 5)] { - eager.update(Coupon::pack(slot, value)); - deferred.update(Coupon::pack(slot, value)); - } - assert_eq!(deferred.estimate(), eager.estimate()); - assert_eq!(deferred.serialize(), eager.serialize()); - } - - #[test] - fn interrupted_bulk_merge_cannot_leave_stale_statistics() { - let mut array = Array8::new(4); - array.update(Coupon::pack(0, 1)); - let mut expected = array.clone(); - expected.merge_registers(std::iter::once((1, 5))); - - let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - array.merge_registers((1..3).map(|slot| { - assert_eq!(slot, 1, "interrupted register input"); - (slot, 5) - })); - })); - assert!(result.is_err()); - assert_eq!(array.estimate(), expected.estimate()); - assert_eq!(array.serialize(), expected.serialize()); - } -} diff --git a/datasketches/src/hll/array4.rs b/datasketches/src/hll/array4.rs index 3e7650fd..4190a3f7 100644 --- a/datasketches/src/hll/array4.rs +++ b/datasketches/src/hll/array4.rs @@ -19,12 +19,12 @@ use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; +use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; -use crate::hll::array::HllArray; -use crate::hll::array::Registers; use crate::hll::aux_map::AuxMap; -use crate::hll::estimator::Kxq; +use crate::hll::estimator::EstimateState; +use crate::hll::estimator::Estimator; use crate::hll::serialization::COMPACT_FLAG_MASK; use crate::hll::serialization::COUPON_SIZE_BYTES; use crate::hll::serialization::CUR_MODE_HLL; @@ -55,7 +55,7 @@ impl AuxFormat { /// Registers stored as four-bit offsets from their current minimum. #[derive(Debug, Clone, PartialEq)] -pub struct Registers4 { +pub struct Array4 { lg_config_k: u8, /// Even slots occupy the low nibble; odd slots occupy the high nibble. bytes: Box<[u8]>, @@ -64,11 +64,23 @@ pub struct Registers4 { num_at_cur_min: u32, /// Every `AUX_TOKEN` nibble has an entry here containing the absolute register value. aux_map: Option, + estimator: Estimator, } -pub type Array4 = HllArray; +impl Array4 { + pub fn new(lg_config_k: u8) -> Self { + let num_bytes = 1 << (lg_config_k - 1); + let num_at_cur_min = 1 << lg_config_k; + Self { + lg_config_k, + bytes: vec![0u8; num_bytes].into_boxed_slice(), + cur_min: 0, + num_at_cur_min, + aux_map: None, + estimator: Estimator::new(lg_config_k), + } + } -impl Registers4 { #[inline] fn get_raw(&self, slot: u32) -> u8 { debug_assert!(slot >> 1 < self.bytes.len() as u32); @@ -77,6 +89,27 @@ impl Registers4 { if slot & 1 == 0 { byte & 15 } else { byte >> 4 } } + pub fn get(&self, slot: u32) -> u8 { + let raw = self.get_raw(slot); + + if raw < AUX_TOKEN { + self.cur_min + raw + } else { + self.aux_map + .as_ref() + .and_then(|map| map.get(slot)) + .unwrap_or(self.cur_min) + } + } + + pub fn num_registers(&self) -> usize { + 1 << self.lg_config_k + } + + pub fn estimate_state(&self) -> EstimateState { + self.estimator.estimate_state() + } + #[inline] fn put_raw(&mut self, slot: u32, value: u8) { debug_assert!(value <= AUX_TOKEN); @@ -91,106 +124,20 @@ impl Registers4 { }; } - /// Requires no registers at `cur_min`. Raising the offset makes some auxiliary values - /// fit in the nibbles again; the logical register values must stay unchanged. - fn shift_to_bigger_cur_min(&mut self) { - let new_cur_min = self.cur_min + 1; - let k = 1 << self.lg_config_k; - let mut num_at_new = 0; - - for slot in 0..k { - let raw = self.get_raw(slot); - debug_assert_ne!(raw, 0, "value cannot be 0 when shifting cur_min"); - if raw < AUX_TOKEN { - let decremented = raw - 1; - self.put_raw(slot, decremented); - if decremented == 0 { - num_at_new += 1; - } - } - } - - // Rebuild aux map: some exceptions may no longer be exceptions - if let Some(old_aux) = self.aux_map.take() { - let mut new_aux = None; - - for (slot, old_actual_val) in old_aux.into_iter() { - debug_assert_eq!( - self.get_raw(slot), - AUX_TOKEN, - "AuxMap contains slot != AUX_TOKEN" - ); - - let new_shifted = old_actual_val - new_cur_min; - - if new_shifted < AUX_TOKEN { - self.put_raw(slot, new_shifted); - } else { - let aux = new_aux.get_or_insert_with(|| AuxMap::new(self.lg_config_k)); - aux.insert(slot, old_actual_val); - } - } - self.aux_map = new_aux; - } - - self.cur_min = new_cur_min; - self.num_at_cur_min = num_at_new; - } -} - -impl Registers for Registers4 { - type Statistics = Kxq; - fn new(lg_config_k: u8) -> Self { - let num_bytes = 1 << (lg_config_k - 1); - let num_at_cur_min = 1 << lg_config_k; - Self { - lg_config_k, - bytes: vec![0u8; num_bytes].into_boxed_slice(), - cur_min: 0, - num_at_cur_min, - aux_map: None, - } - } - - fn lg_config_k(&self) -> u8 { - self.lg_config_k - } - - fn get(&self, slot: u32) -> u8 { - read_register(&self.bytes, self.cur_min, self.aux_map.as_ref(), slot) - } - - fn values(&self) -> impl Iterator { - (0..1 << self.lg_config_k).map(|slot| self.get(slot)) - } - - fn num_zeros(&self) -> u32 { - if self.cur_min == 0 { - self.num_at_cur_min - } else { - 0 - } - } - - fn is_empty(&self) -> bool { - self.num_zeros() == (1 << self.lg_config_k) - } - - #[inline] - fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { + pub fn update(&mut self, coupon: Coupon) { let mask = (1 << self.lg_config_k) - 1; let slot = coupon.slot() & mask; let new_value = coupon.value(); if new_value <= self.cur_min { - return None; + return; } let raw_stored = self.get_raw(slot); let lower_bound = raw_stored + self.cur_min; if new_value <= lower_bound { - return None; + return; } let old_value = if raw_stored < AUX_TOKEN { @@ -204,9 +151,12 @@ impl Registers for Registers4 { }; if new_value <= old_value { - return None; + return; } + self.estimator + .update(self.lg_config_k, old_value, new_value); + let shifted_new = new_value - self.cur_min; match (raw_stored, shifted_new) { @@ -238,11 +188,90 @@ impl Registers for Registers4 { self.shift_to_bigger_cur_min(); } } - Some((old_value, new_value)) } -} -impl Array4 { + /// Requires no registers at `cur_min`. Raising the offset makes some auxiliary values + /// fit in the nibbles again; the logical register values must stay unchanged. + fn shift_to_bigger_cur_min(&mut self) { + let new_cur_min = self.cur_min + 1; + let k = 1 << self.lg_config_k; + let mut num_at_new = 0; + + for slot in 0..k { + let raw = self.get_raw(slot); + debug_assert_ne!(raw, 0, "value cannot be 0 when shifting cur_min"); + if raw < AUX_TOKEN { + let decremented = raw - 1; + self.put_raw(slot, decremented); + if decremented == 0 { + num_at_new += 1; + } + } + } + + // Rebuild aux map: some exceptions may no longer be exceptions + if let Some(old_aux) = self.aux_map.take() { + let mut new_aux = None; + + for (slot, old_actual_val) in old_aux.into_iter() { + debug_assert_eq!( + self.get_raw(slot), + AUX_TOKEN, + "AuxMap contains slot != AUX_TOKEN" + ); + + let new_shifted = old_actual_val - new_cur_min; + + if new_shifted < AUX_TOKEN { + self.put_raw(slot, new_shifted); + } else { + let aux = new_aux.get_or_insert_with(|| AuxMap::new(self.lg_config_k)); + aux.insert(slot, old_actual_val); + } + } + self.aux_map = new_aux; + } + + self.cur_min = new_cur_min; + self.num_at_cur_min = num_at_new; + } + + pub fn estimate(&self) -> f64 { + self.estimator + .estimate(self.lg_config_k, self.cur_min, self.num_at_cur_min) + } + + pub fn composite_estimate(&self) -> f64 { + self.estimator + .composite_estimate(self.lg_config_k, self.cur_min, self.num_at_cur_min) + } + + pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { + self.estimator.upper_bound( + self.lg_config_k, + self.cur_min, + self.num_at_cur_min, + num_std_dev, + ) + } + + pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { + self.estimator.lower_bound( + self.lg_config_k, + self.cur_min, + self.num_at_cur_min, + num_std_dev, + ) + } + + pub fn restore_estimate_state(&mut self, state: EstimateState) { + self.estimator.restore_estimate_state(state); + } + + pub fn is_empty(&self) -> bool { + self.num_at_cur_min == (1 << self.lg_config_k) && self.cur_min == 0 + } + pub fn deserialize( mut cursor: SketchSlice, cur_min: u8, @@ -331,30 +360,23 @@ impl Array4 { aux_map = Some(aux); } - Ok(Self::from_serialized( - Registers4 { - lg_config_k, - bytes: data.into_boxed_slice(), - cur_min, - num_at_cur_min, - aux_map, - }, - hip_accum, - kxq0, - kxq1, - if cur_min == 0 { num_at_cur_min } else { 0 }, - ooo, - )) + let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + + Ok(Self { + lg_config_k, + bytes: data.into_boxed_slice(), + cur_min, + num_at_cur_min, + aux_map, + estimator, + }) } /// Writes packed registers followed by compact auxiliary entries. - pub fn serialize(&self) -> Vec { - let registers = self.registers(); - let lg_config_k = self.lg_config_k(); - let statistics = self.statistics(); + pub fn serialize(&self, lg_config_k: u8) -> Vec { let num_bytes = 1 << (lg_config_k - 1); - let aux_entries: Vec<(u32, u8)> = if let Some(aux) = ®isters.aux_map { + let aux_entries: Vec<(u32, u8)> = if let Some(aux) = &self.aux_map { aux.iter().collect() } else { vec![] @@ -371,24 +393,24 @@ impl Array4 { bytes.write_u8(0); // lg_arr is unused for compact auxiliary storage. let mut flags = COMPACT_FLAG_MASK; - if self.estimate_state().uses_composite_estimate() { + if self.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); - bytes.write_u8(registers.cur_min); + bytes.write_u8(self.cur_min); bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL4)); - bytes.write_f64_le(self.estimate_state().hip_accum()); - bytes.write_f64_le(statistics.kxq.low); - bytes.write_f64_le(statistics.kxq.high); + bytes.write_f64_le(self.estimator.hip_accum()); + bytes.write_f64_le(self.estimator.kxq0()); + bytes.write_f64_le(self.estimator.kxq1()); - bytes.write_u32_le(registers.num_at_cur_min); + bytes.write_u32_le(self.num_at_cur_min); bytes.write_u32_le(aux_count); - bytes.write(®isters.bytes); + bytes.write(&self.bytes); for (slot, value) in aux_entries.iter().copied() { bytes.write_u32_le(Coupon::pack(slot, value).raw()); @@ -399,9 +421,8 @@ impl Array4 { /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.registers().bytes.len() + self.bytes.len() + self - .registers() .aux_map .as_ref() .map(|a| a.estimated_size()) @@ -409,17 +430,6 @@ impl Array4 { } } -fn read_register(bytes: &[u8], cur_min: u8, aux_map: Option<&AuxMap>, slot: u32) -> u8 { - let byte = bytes[(slot >> 1) as usize]; - let raw = if slot & 1 == 0 { byte & 15 } else { byte >> 4 }; - - if raw < AUX_TOKEN { - cur_min + raw - } else { - aux_map.and_then(|map| map.get(slot)).unwrap_or(cur_min) - } -} - #[cfg(test)] mod tests { use googletest::assert_that; @@ -433,7 +443,7 @@ mod tests { #[test] fn test_get_set_raw() { - let mut data = Registers4::new(4); // 16 buckets + let mut data = Array4::new(4); // 16 buckets // Test even slot (low nibble) data.put_raw(0, 5); @@ -481,11 +491,11 @@ mod tests { // kxq0 should have decreased (we removed a 0 and added a 10) // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.statistics().kxq.low, lt(256.0)); + assert_that!(arr.estimator.kxq0(), lt(256.0)); // kxq1 should have a small positive value (from 1/2^40) - assert_that!(arr.statistics().kxq.high, gt(0.0)); - assert_that!(arr.statistics().kxq.high, lt(0.001)); + assert_that!(arr.estimator.kxq1(), gt(0.0)); + assert_that!(arr.estimator.kxq1(), lt(0.001)); } #[test] @@ -495,21 +505,18 @@ mod tests { let mut arr = Array4::new(lg_config_k); arr.update(Coupon::pack(0, 15)); - assert_eq!(arr.registers().get_raw(0), AUX_TOKEN); - assert_eq!( - arr.registers().aux_map.as_ref().and_then(|aux| aux.get(0)), - Some(15) - ); + assert_eq!(arr.get_raw(0), AUX_TOKEN); + assert_eq!(arr.aux_map.as_ref().and_then(|aux| aux.get(0)), Some(15)); for slot in 1..num_slots { arr.update(Coupon::pack(slot, 1)); } - assert_eq!(arr.registers().cur_min, 1); - assert_eq!(arr.registers().num_at_cur_min, num_slots - 1); - assert_eq!(arr.registers().get_raw(0), 14); + assert_eq!(arr.cur_min, 1); + assert_eq!(arr.num_at_cur_min, num_slots - 1); + assert_eq!(arr.get_raw(0), 14); assert_eq!(arr.get(0), 15); - assert_that!(arr.registers().aux_map, none()); + assert_that!(arr.aux_map, none()); for slot in 1..num_slots { assert_eq!(arr.get(slot), 1); diff --git a/datasketches/src/hll/array6.rs b/datasketches/src/hll/array6.rs index 05143158..636da15d 100644 --- a/datasketches/src/hll/array6.rs +++ b/datasketches/src/hll/array6.rs @@ -19,11 +19,11 @@ use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; +use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; -use crate::hll::array::HllArray; -use crate::hll::array::Registers; -use crate::hll::estimator::Kxq; +use crate::hll::estimator::EstimateState; +use crate::hll::estimator::Estimator; use crate::hll::serialization::CUR_MODE_HLL; use crate::hll::serialization::HLL_PREAMBLE_SIZE; use crate::hll::serialization::HLL_PREINTS; @@ -36,18 +36,48 @@ const VAL_MASK_6: u16 = 0x3F; /// Registers packed into consecutive six-bit fields. #[derive(Debug, Clone, PartialEq)] -pub struct Registers6 { +pub struct Array6 { lg_config_k: u8, bytes: Box<[u8]>, num_zeros: u32, + estimator: Estimator, } -pub type Array6 = HllArray; +impl Array6 { + pub fn new(lg_config_k: u8) -> Self { + let k = 1 << lg_config_k; + let num_bytes = num_bytes_for_k(k); + + Self { + lg_config_k, + bytes: vec![0u8; num_bytes].into_boxed_slice(), + num_zeros: k, + estimator: Estimator::new(lg_config_k), + } + } -impl Registers6 { #[inline] fn get_raw(&self, slot: u32) -> u8 { - read_register(&self.bytes, slot) + let start_bit = slot * 6; + let byte_idx = (start_bit >> 3) as usize; + let shift = (start_bit & 7) as u8; + + let two_bytes = u16::from_le_bytes([self.bytes[byte_idx], self.bytes[byte_idx + 1]]); + + ((two_bytes >> shift) & VAL_MASK_6) as u8 + } + + #[inline] + pub fn get(&self, slot: u32) -> u8 { + self.get_raw(slot) + } + + pub fn num_registers(&self) -> usize { + 1 << self.lg_config_k + } + + pub fn estimate_state(&self) -> EstimateState { + self.estimator.estimate_state() } #[inline] @@ -68,62 +98,54 @@ impl Registers6 { self.bytes[byte_idx] = bytes_out[0]; self.bytes[byte_idx + 1] = bytes_out[1]; } -} -impl Registers for Registers6 { - type Statistics = Kxq; - fn new(lg_config_k: u8) -> Self { - let k = 1 << lg_config_k; - let num_bytes = num_bytes_for_k(k); + pub fn update(&mut self, coupon: Coupon) { + let mask = (1 << self.lg_config_k) - 1; + let slot = coupon.slot() & mask; + let new_value = coupon.value(); - Self { - lg_config_k, - bytes: vec![0u8; num_bytes].into_boxed_slice(), - num_zeros: k, + let old_value = self.get_raw(slot); + + if new_value > old_value { + self.estimator + .update(self.lg_config_k, old_value, new_value); + + self.put_raw(slot, new_value); + + if old_value == 0 { + self.num_zeros -= 1; + } } } - fn lg_config_k(&self) -> u8 { - self.lg_config_k + pub fn estimate(&self) -> f64 { + // Array6 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros + self.estimator.estimate(self.lg_config_k, 0, self.num_zeros) } - fn get(&self, slot: u32) -> u8 { - self.get_raw(slot) + pub fn composite_estimate(&self) -> f64 { + self.estimator + .composite_estimate(self.lg_config_k, 0, self.num_zeros) } - fn values(&self) -> impl Iterator { - (0..1 << self.lg_config_k).map(|slot| self.get(slot)) + pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { + self.estimator + .upper_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) } - fn num_zeros(&self) -> u32 { - self.num_zeros + pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { + self.estimator + .lower_bound(self.lg_config_k, 0, self.num_zeros, num_std_dev) } - fn is_empty(&self) -> bool { - self.num_zeros() == (1 << self.lg_config_k) + pub fn restore_estimate_state(&mut self, state: EstimateState) { + self.estimator.restore_estimate_state(state); } - #[inline] - fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { - let mask = (1 << self.lg_config_k) - 1; - let slot = coupon.slot() & mask; - let new_value = coupon.value(); - - let old_value = self.get_raw(slot); - - if new_value > old_value { - self.put_raw(slot, new_value); - if old_value == 0 { - self.num_zeros -= 1; - } - Some((old_value, new_value)) - } else { - None - } + pub fn is_empty(&self) -> bool { + self.num_zeros == (1 << self.lg_config_k) } -} -impl Array6 { pub fn deserialize_registers( mut cursor: SketchSlice, lg_config_k: u8, @@ -163,24 +185,17 @@ impl Array6 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - Ok(Self::from_serialized( - Registers6 { - lg_config_k, - bytes: data.into_boxed_slice(), - num_zeros, - }, - hip_accum, - kxq0, - kxq1, + let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + + Ok(Self { + lg_config_k, + bytes: data.into_boxed_slice(), num_zeros, - ooo, - )) + estimator, + }) } - pub fn serialize(&self) -> Vec { - let registers = self.registers(); - let lg_config_k = self.lg_config_k(); - let statistics = self.statistics(); + pub fn serialize(&self, lg_config_k: u8) -> Vec { let k = 1 << lg_config_k; let num_bytes = num_bytes_for_k(k); let total_size = HLL_PREAMBLE_SIZE + num_bytes; @@ -193,7 +208,7 @@ impl Array6 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if self.estimate_state().uses_composite_estimate() { + if self.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -203,37 +218,27 @@ impl Array6 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL6)); - bytes.write_f64_le(self.estimate_state().hip_accum()); - bytes.write_f64_le(statistics.kxq.low); - bytes.write_f64_le(statistics.kxq.high); + bytes.write_f64_le(self.estimator.hip_accum()); + bytes.write_f64_le(self.estimator.kxq0()); + bytes.write_f64_le(self.estimator.kxq1()); // Write num_at_cur_min (num_zeros for Array6) - bytes.write_u32_le(statistics.num_zeros); + bytes.write_u32_le(self.num_zeros); // Write aux_count (always 0 for Array6) bytes.write_u32_le(0); - bytes.write(®isters.bytes); + bytes.write(&self.bytes); bytes.into_bytes() } /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.registers().bytes.len() + self.bytes.len() } } -fn read_register(bytes: &[u8], slot: u32) -> u8 { - let start_bit = slot * 6; - let byte_idx = (start_bit >> 3) as usize; - let shift = (start_bit & 7) as u8; - - let two_bytes = u16::from_le_bytes([bytes[byte_idx], bytes[byte_idx + 1]]); - - ((two_bytes >> shift) & VAL_MASK_6) as u8 -} - fn num_bytes_for_k(k: u32) -> usize { // The final register's 16-bit read/write window needs one padding byte. (((k * 3) >> 2) + 1) as usize @@ -258,7 +263,7 @@ mod tests { #[test] fn test_get_set_raw() { - let mut arr = Registers6::new(4); // 16 slots + let mut arr = Array6::new(4); // 16 slots arr.put_raw(0, 0); arr.put_raw(1, 1); @@ -285,7 +290,7 @@ mod tests { #[test] fn test_boundary_crossing() { - let mut arr = Registers6::new(8); // 256 slots + let mut arr = Array6::new(8); // 256 slots // Slot 1: starts at bit 6 (crosses byte 0/1 boundary) arr.put_raw(1, 0b111111); @@ -325,7 +330,7 @@ mod tests { #[test] fn test_full_range() { - let mut arr = Registers6::new(6); // 64 slots + let mut arr = Array6::new(6); // 64 slots for val in 0..64u8 { arr.put_raw(val as u32, val); @@ -344,10 +349,10 @@ mod tests { arr.update(Coupon::pack(1, 40)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.statistics().kxq.low, lt(256.0)); + assert_that!(arr.estimator.kxq0(), lt(256.0)); // kxq1 should have a small positive value (from 1/2^40) - assert_that!(arr.statistics().kxq.high, gt(0.0)); - assert_that!(arr.statistics().kxq.high, lt(0.001)); + assert_that!(arr.estimator.kxq1(), gt(0.0)); + assert_that!(arr.estimator.kxq1(), lt(0.001)); } } diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index 693e5831..c111b025 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -21,11 +21,11 @@ use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; use crate::codec::family::Family; +use crate::common::NumStdDev; use crate::error::Error; use crate::hll::Coupon; -use crate::hll::array::HllArray; -use crate::hll::array::Registers; -use crate::hll::estimator::RegisterStatistics; +use crate::hll::estimator::EstimateState; +use crate::hll::estimator::Estimator; use crate::hll::serialization::CUR_MODE_HLL; use crate::hll::serialization::HLL_PREAMBLE_SIZE; use crate::hll::serialization::HLL_PREINTS; @@ -35,84 +35,212 @@ use crate::hll::serialization::TGT_HLL8; use crate::hll::serialization::encode_mode_byte; /// Registers stored one per byte. -#[derive(Debug, Clone, PartialEq)] -pub struct Registers8 { +#[derive(Debug, Clone)] +pub struct Array8 { lg_config_k: u8, bytes: Box<[u8]>, + /// An absent cache means a bulk merge invalidated HIP and the register-derived fields. + /// Shared queries initialize it once; exclusive updates can maintain or invalidate it. + cache: OnceLock, +} + +#[derive(Debug, Clone, PartialEq)] +struct CachedState { + num_zeros: u32, + estimator: Estimator, } -pub type Array8 = HllArray; +impl PartialEq for Array8 { + fn eq(&self, other: &Self) -> bool { + self.lg_config_k == other.lg_config_k + && self.bytes == other.bytes + && self.cached_state() == other.cached_state() + } +} + +impl Array8 { + pub fn new(lg_config_k: u8) -> Self { + let k = 1 << lg_config_k; -impl Registers for Registers8 { - type Statistics = OnceLock; - fn new(lg_config_k: u8) -> Self { Self { lg_config_k, - bytes: vec![0; 1 << lg_config_k].into_boxed_slice(), + bytes: vec![0u8; k as usize].into_boxed_slice(), + cache: OnceLock::from(CachedState { + num_zeros: k, + estimator: Estimator::new(lg_config_k), + }), } } - fn lg_config_k(&self) -> u8 { - self.lg_config_k + #[inline] + pub fn get(&self, slot: u32) -> u8 { + self.bytes[slot as usize] } - fn get(&self, slot: u32) -> u8 { - self.bytes[slot as usize] + #[inline] + fn put(&mut self, slot: u32, value: u8) { + self.bytes[slot as usize] = value; } - fn values(&self) -> impl Iterator { - self.bytes.iter().copied() + pub fn update(&mut self, coupon: Coupon) { + let mask = (1 << self.lg_config_k) - 1; + let slot = coupon.slot() & mask; + let new_value = coupon.value(); + + let old_value = self.get(slot); + + if new_value > old_value { + if let Some(cache) = self.cache.get_mut() { + cache + .estimator + .update(self.lg_config_k, old_value, new_value); + if old_value == 0 { + cache.num_zeros -= 1; + } + } + self.put(slot, new_value); + } } - fn num_zeros(&self) -> u32 { - self.bytes.iter().filter(|&&value| value == 0).count() as u32 + pub fn estimate(&self) -> f64 { + let cache = self.cached_state(); + // Array8 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros + cache + .estimator + .estimate(self.lg_config_k, 0, cache.num_zeros) } - fn is_empty(&self) -> bool { - self.bytes.iter().all(|&value| value == 0) + pub fn composite_estimate(&self) -> f64 { + let cache = self.cached_state(); + cache + .estimator + .composite_estimate(self.lg_config_k, 0, cache.num_zeros) } - #[inline] - fn update(&mut self, coupon: Coupon) -> Option<(u8, u8)> { - let slot = (coupon.slot() & ((1 << self.lg_config_k) - 1)) as usize; - let old = self.bytes[slot]; - let new = coupon.value(); - if new > old { - self.bytes[slot] = new; - Some((old, new)) - } else { - None + pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { + let cache = self.cached_state(); + cache + .estimator + .upper_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + } + + pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { + let cache = self.cached_state(); + cache + .estimator + .lower_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + } + + pub fn is_empty(&self) -> bool { + if let Some(cache) = self.cache.get() { + return cache.num_zeros == (1 << self.lg_config_k); } + // Union checks emptiness between merges. A nonzero register answers that without + // rebuilding all estimator fields and defeating deferred reconstruction. + self.bytes.iter().all(|&value| value == 0) } -} -impl Registers8 { - pub fn as_slice(&self) -> &[u8] { + pub fn values(&self) -> &[u8] { &self.bytes } - pub fn merge_registers(&mut self, values: impl Iterator) { - for (slot, value) in values { - self.bytes[slot] = self.bytes[slot].max(value); + pub fn num_registers(&self) -> usize { + 1 << self.lg_config_k + } + + pub fn estimate_state(&self) -> EstimateState { + self.cache.get().map_or(EstimateState::Composite, |cache| { + cache.estimator.estimate_state() + }) + } + + pub fn restore_estimate_state(&mut self, state: EstimateState) { + if let Some(cache) = self.cache.get_mut() { + cache.estimator.restore_estimate_state(state); + } else if let EstimateState::Hip(_) = state { + let mut cache = self.rebuild_cached_values(); + cache.estimator.restore_estimate_state(state); + self.cache = OnceLock::from(cache); + } + } + + /// Bypasses estimator updates. Call `invalidate_estimator` after all writes. + pub fn set_register(&mut self, slot: usize, value: u8) { + self.bytes[slot] = value; + } + + /// Invalidates HIP and defers rebuilding register-derived fields until a query needs them. + pub fn invalidate_estimator(&mut self) { + self.cache.take(); + } + + /// Merges registers and invalidates HIP because their update order is unavailable. + pub fn merge_array_same_lgk(&mut self, src: &[u8]) { + assert_eq!( + src.len(), + self.bytes.len(), + "Source and destination must have same lg_k" + ); + + for (i, &val) in src.iter().enumerate() { + self.bytes[i] = self.bytes[i].max(val); } + + self.invalidate_estimator(); } - pub fn merge_same_lgk(&mut self, values: &[u8]) { - for (dst, &src) in self.bytes.iter_mut().zip(values) { - *dst = (*dst).max(src); + /// Register values come from a separate hash word, so folding slot indices needs no + /// adjustment to the values. Merging independent histories invalidates HIP. + pub fn merge_array_with_downsample(&mut self, src: &[u8], src_lg_k: u8) { + assert!( + src_lg_k > self.lg_config_k, + "Source lg_k must be greater than destination lg_k for downsampling" + ); + assert_eq!( + src.len(), + 1 << src_lg_k, + "Source length must match 2^src_lg_k" + ); + + let dst_mask = (1 << self.lg_config_k) - 1; + + for (src_slot, &val) in src.iter().enumerate() { + let dst_slot = (src_slot as u32 & dst_mask) as usize; + self.bytes[dst_slot] = self.bytes[dst_slot].max(val); } + + self.invalidate_estimator(); } - pub fn merge_with_downsample(&mut self, values: &[u8]) { - let mask = (1 << self.lg_config_k) - 1; - for (src_slot, &value) in values.iter().enumerate() { - let dst_slot = (src_slot as u32 & mask) as usize; - self.bytes[dst_slot] = self.bytes[dst_slot].max(value); + fn cached_state(&self) -> &CachedState { + self.cache.get_or_init(|| self.rebuild_cached_values()) + } + + fn rebuild_cached_values(&self) -> CachedState { + let num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; + + let mut kxq0_sum = 0.0; + let mut kxq1_sum = 0.0; + + for &val in self.bytes.iter() { + if val == 0 { + kxq0_sum += 1.0; + } else if val < 32 { + kxq0_sum += 1.0 / (1u64 << val) as f64; + } else { + kxq1_sum += 1.0 / (1u64 << val) as f64; + } + } + + let mut estimator = Estimator::new(self.lg_config_k); + estimator.restore_kxq(kxq0_sum, kxq1_sum); + estimator.invalidate_hip(); + CachedState { + num_zeros, + estimator, } } -} -impl Array8 { pub fn deserialize_registers( mut cursor: SketchSlice, lg_config_k: u8, @@ -151,22 +279,20 @@ impl Array8 { .read_exact(&mut data) .map_err(insufficient_data("data"))?; - Ok(Self::from_serialized( - Registers8 { - lg_config_k, - bytes: data.into_boxed_slice(), - }, - hip_accum, - kxq0, - kxq1, - num_zeros, - ooo, - )) + let estimator = Estimator::from_serialized(hip_accum, kxq0, kxq1, ooo); + + Ok(Self { + lg_config_k, + bytes: data.into_boxed_slice(), + cache: OnceLock::from(CachedState { + num_zeros, + estimator, + }), + }) } - pub fn serialize(&self) -> Vec { - let lg_config_k = self.lg_config_k(); - let statistics = self.statistics(); + pub fn serialize(&self, lg_config_k: u8) -> Vec { + let cache = self.cached_state(); let k = 1 << lg_config_k; let total_size = HLL_PREAMBLE_SIZE + k as usize; let mut bytes = SketchBytes::with_capacity(total_size); @@ -178,7 +304,7 @@ impl Array8 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if self.estimate_state().uses_composite_estimate() { + if cache.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -188,23 +314,24 @@ impl Array8 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL8)); - bytes.write_f64_le(self.estimate_state().hip_accum()); - bytes.write_f64_le(statistics.kxq.low); - bytes.write_f64_le(statistics.kxq.high); + bytes.write_f64_le(cache.estimator.hip_accum()); + bytes.write_f64_le(cache.estimator.kxq0()); + bytes.write_f64_le(cache.estimator.kxq1()); // Write num_at_cur_min (num_zeros for Array8) - bytes.write_u32_le(statistics.num_zeros); + bytes.write_u32_le(cache.num_zeros); // Write aux_count (always 0 for Array8) bytes.write_u32_le(0); - bytes.write(self.values()); + bytes.write(&self.bytes); bytes.into_bytes() } + /// Returns an estimate of memory usage in bytes. pub fn estimated_size(&self) -> usize { - self.values().len() + self.bytes.len() } } @@ -229,19 +356,19 @@ mod tests { #[test] fn test_get_set() { - let mut arr = Registers8::new(4); // 16 slots + let mut arr = Array8::new(4); // 16 slots for slot in 0..16 { - arr.bytes[slot] = (slot * 17) as u8; + arr.put(slot, (slot * 17) as u8); } for slot in 0..16 { assert_eq!(arr.get(slot), (slot * 17) as u8); } - arr.bytes[0] = 0; - arr.bytes[1] = 127; - arr.bytes[2] = 255; + arr.put(0, 0); + arr.put(1, 127); + arr.put(2, 255); assert_eq!(arr.get(0), 0); assert_eq!(arr.get(1), 127); @@ -286,10 +413,10 @@ mod tests { #[test] fn test_full_value_range() { - let mut arr = Registers8::new(8); // 256 slots + let mut arr = Array8::new(8); // 256 slots for val in 0..=255u8 { - arr.bytes[val as usize] = val; + arr.put(val as u32, val); } for val in 0..=255u8 { @@ -299,13 +426,13 @@ mod tests { #[test] fn test_high_value_direct() { - let mut arr = Registers8::new(6); // 64 slots + let mut arr = Array8::new(6); // 64 slots // Direct storage accepts byte values beyond the six-bit coupon range. let test_values = [16, 32, 64, 128, 200, 255]; for (slot, &value) in test_values.iter().enumerate() { - arr.bytes[slot] = value; + arr.put(slot as u32, value); assert_eq!(arr.get(slot as u32), value); } @@ -322,20 +449,20 @@ mod tests { arr.update(Coupon::pack(1, 50)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.statistics().kxq.low, lt(256.0)); + assert_that!(arr.cached_state().estimator.kxq0(), lt(256.0)); // kxq1 should have a positive value (from 1/2^50) - assert_that!(arr.statistics().kxq.high, gt(0.0)); - assert_that!(arr.statistics().kxq.high, lt(1e-10)); + assert_that!(arr.cached_state().estimator.kxq1(), gt(0.0)); + assert_that!(arr.cached_state().estimator.kxq1(), lt(1e-10)); } #[test] fn test_values_access() { let mut arr = Array8::new(4); // 16 slots - arr.merge_registers(std::iter::once((0, 10))); - arr.merge_registers(std::iter::once((5, 25))); - arr.merge_registers(std::iter::once((15, 63))); + arr.put(0, 10); + arr.put(5, 25); + arr.put(15, 63); let vals = arr.values(); assert_eq!(vals.len(), 16); @@ -350,13 +477,13 @@ mod tests { let mut dst = Array8::new(4); // 16 slots let mut src = Array8::new(4); // 16 slots - dst.merge_registers(std::iter::once((0, 10))); - dst.merge_registers(std::iter::once((1, 20))); - dst.merge_registers(std::iter::once((2, 30))); + dst.put(0, 10); + dst.put(1, 20); + dst.put(2, 30); - src.merge_registers(std::iter::once((1, 15))); // Smaller than dst[1]=20, should keep 20 - src.merge_registers(std::iter::once((2, 35))); // Larger than dst[2]=30, should update to 35 - src.merge_registers(std::iter::once((3, 40))); // New value + src.put(1, 15); // Smaller than dst[1]=20, should keep 20 + src.put(2, 35); // Larger than dst[2]=30, should update to 35 + src.put(3, 40); // New value dst.merge_array_same_lgk(src.values()); @@ -366,9 +493,9 @@ mod tests { assert_eq!(dst.get(3), 40, "dst[3] got new value"); // Bulk merges require composite estimation. - assert!(dst.estimate_state().uses_composite_estimate()); + assert!(dst.cached_state().estimator.uses_composite_estimate()); - assert_eq!(dst.statistics().num_zeros, 12); + assert_eq!(dst.cached_state().num_zeros, 12); } #[test] @@ -377,14 +504,14 @@ mod tests { let mut dst = Array8::new(4); // 16 slots let mut src = Array8::new(5); // 32 slots - dst.merge_registers(std::iter::once((0, 10))); - dst.merge_registers(std::iter::once((1, 20))); + dst.put(0, 10); + dst.put(1, 20); // Set up src - slots 0 and 16 both map to dst slot 0 - src.merge_registers(std::iter::once((0, 15))); // maps to dst[0], max(10, 15) = 15 - src.merge_registers(std::iter::once((16, 25))); // maps to dst[0], max(15, 25) = 25 - src.merge_registers(std::iter::once((1, 18))); // maps to dst[1], max(20, 18) = 20 - src.merge_registers(std::iter::once((17, 30))); // maps to dst[1], max(20, 30) = 30 + src.put(0, 15); // maps to dst[0], max(10, 15) = 15 + src.put(16, 25); // maps to dst[0], max(15, 25) = 25 + src.put(1, 18); // maps to dst[1], max(20, 18) = 20 + src.put(17, 30); // maps to dst[1], max(20, 30) = 30 dst.merge_array_with_downsample(src.values(), 5); @@ -392,7 +519,7 @@ mod tests { assert_eq!(dst.get(1), 30, "dst[1] = max(20, 18, 30)"); // Bulk merges require composite estimation. - assert!(dst.estimate_state().uses_composite_estimate()); + assert!(dst.cached_state().estimator.uses_composite_estimate()); } #[test] @@ -417,21 +544,22 @@ mod tests { fn test_rebuild_cached_values() { let mut arr = Array8::new(4); // 16 slots - arr.merge_registers(std::iter::once((0, 10))); - arr.merge_registers(std::iter::once((1, 20))); - arr.merge_registers(std::iter::once((2, 30))); + arr.put(0, 10); + arr.put(1, 20); + arr.put(2, 30); - arr.merge_registers(std::iter::once((3, 31))); - arr.merge_registers(std::iter::once((4, 32))); - arr.merge_registers(std::iter::once((5, 63))); + arr.put(3, 31); + arr.put(4, 32); + arr.put(5, 63); + arr.invalidate_estimator(); - let statistics = arr.statistics(); - assert_eq!(statistics.num_zeros, 10); + let cache = arr.cached_state(); + assert_eq!(cache.num_zeros, 10); assert_eq!( - statistics.kxq.low, + cache.estimator.kxq0(), 10.0 + 2_f64.powi(-10) + 2_f64.powi(-20) + 2_f64.powi(-30) + 2_f64.powi(-31) ); - assert_eq!(statistics.kxq.high, 2_f64.powi(-32) + 2_f64.powi(-63)); + assert_eq!(cache.estimator.kxq1(), 2_f64.powi(-32) + 2_f64.powi(-63)); } #[test] @@ -440,11 +568,11 @@ mod tests { let mut src = Array8::new(4); for i in 0..16 { - dst.merge_registers(std::iter::once((i as usize, i as u8))); + dst.put(i, i as u8); } for i in 0..16 { - src.merge_registers(std::iter::once((i as usize, (15 - i) as u8))); + src.put(i, (15 - i) as u8); } dst.merge_array_same_lgk(src.values()); diff --git a/datasketches/src/hll/estimator.rs b/datasketches/src/hll/estimator.rs index 9b672ba4..a72fcca0 100644 --- a/datasketches/src/hll/estimator.rs +++ b/datasketches/src/hll/estimator.rs @@ -15,7 +15,10 @@ // specific language governing permissions and limitations // under the License. -//! Cardinality estimation from HLL register statistics and update history. +//! Cardinality estimation for HLL-mode sketches. +//! +//! Sequential register updates use HIP (Historical Inverse Probability). Bulk merges use the +//! composite estimator because register values do not retain their update order. //! //! Estimator formulas and crossover constants follow datasketches-cpp: //! @@ -35,86 +38,119 @@ pub enum EstimateState { Composite, } -impl EstimateState { - pub fn from_serialized(hip_accum: f64, out_of_order: bool) -> Self { - if out_of_order { - Self::Composite - } else { - Self::Hip(hip_accum) +/// KxQ caches the sum of `2^-register`. Splitting the sum at register value 32 preserves +/// precision when large and small contributions coexist. +#[derive(Debug, Clone, PartialEq)] +pub struct Estimator { + estimate_state: EstimateState, + /// KxQ register for values < 32 (larger inverse powers) + kxq0: f64, + /// KxQ register for values >= 32 (tiny inverse powers) + kxq1: f64, +} + +impl Estimator { + pub fn new(lg_config_k: u8) -> Self { + let k = 1 << lg_config_k; + Self { + estimate_state: EstimateState::Hip(0.0), + kxq0: k as f64, // All registers start at 0, so kxq0 = k * (1/2^0) = k + kxq1: 0.0, } } - pub fn hip_accum(self) -> f64 { - match self { - Self::Hip(value) => value, - Self::Composite => 0.0, + /// Restores estimator fields from an HLL serialization preamble. + pub fn from_serialized(hip_accum: f64, kxq0: f64, kxq1: f64, out_of_order: bool) -> Self { + let estimate_state = if out_of_order { + EstimateState::Composite + } else { + EstimateState::Hip(hip_accum) + }; + Self { + estimate_state, + kxq0, + kxq1, } } - pub fn uses_composite_estimate(self) -> bool { - matches!(self, Self::Composite) - } + /// Applies a register increase; requires `new_value > old_value`. + pub fn update(&mut self, lg_config_k: u8, old_value: u8, new_value: u8) { + let k = (1 << lg_config_k) as f64; - #[inline] - pub fn update(&mut self, lg_config_k: u8, kxq: &mut Kxq, old_value: u8, new_value: u8) { - // HIP uses the sums from before this register increase. - if let Self::Hip(hip_accum) = self { - *hip_accum += (1 << lg_config_k) as f64 / (kxq.low + kxq.high); + // HIP must use the KxQ sums from before this register update. + if let EstimateState::Hip(hip_accum) = &mut self.estimate_state { + *hip_accum += k / (self.kxq0 + self.kxq1); } - kxq.update(old_value, new_value); - } -} -/// Sums of `2^-register`, split at register value 32 to preserve precision. -#[derive(Debug, Clone, Copy, PartialEq)] -pub struct Kxq { - pub low: f64, - pub high: f64, -} + // Always update KxQ because it depends only on the current registers. + self.update_kxq(old_value, new_value); + } -impl Kxq { - #[inline] - fn update(&mut self, old_value: u8, new_value: u8) { + fn update_kxq(&mut self, old_value: u8, new_value: u8) { if old_value < 32 { - self.low -= inv_pow2(old_value); + self.kxq0 -= inv_pow2(old_value); } else { - self.high -= inv_pow2(old_value); + self.kxq1 -= inv_pow2(old_value); } + if new_value < 32 { - self.low += inv_pow2(new_value); + self.kxq0 += inv_pow2(new_value); } else { - self.high += inv_pow2(new_value); + self.kxq1 += inv_pow2(new_value); } } -} - -/// Representation-independent inputs to composite estimation. -#[derive(Debug, Clone, Copy, PartialEq)] -pub struct RegisterStatistics { - pub num_zeros: u32, - pub kxq: Kxq, -} -impl RegisterStatistics { - pub fn from_registers(registers: impl Iterator) -> Self { - let mut statistics = Self { - num_zeros: 0, - kxq: Kxq { - low: 0.0, - high: 0.0, - }, - }; - for value in registers { - if value == 0 { - statistics.num_zeros += 1; - statistics.kxq.low += 1.0; - } else if value < 32 { - statistics.kxq.low += 1.0 / (1u64 << value) as f64; - } else { - statistics.kxq.high += 1.0 / (1u64 << value) as f64; + /// For `Array6`/`Array8`, pass zero for `cur_min` and the zero-register count for + /// `num_at_cur_min`. `Array4` tracks its actual minimum and its count. + pub fn estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { + match self.estimate_state { + EstimateState::Hip(hip_accum) => hip_accum, + EstimateState::Composite => { + self.composite_estimate(lg_config_k, cur_min, num_at_cur_min) } } - statistics + } + + pub fn upper_bound( + &self, + lg_config_k: u8, + cur_min: u8, + num_at_cur_min: u32, + num_std_dev: NumStdDev, + ) -> f64 { + let estimate = self.estimate(lg_config_k, cur_min, num_at_cur_min); + let rse = relative_error( + lg_config_k, + true, + self.uses_composite_estimate(), + num_std_dev, + ); + estimate / (1.0 + rse) + } + + /// Each nonzero register requires a distinct item, so their count is a hard floor + /// for the lower bound. If `cur_min` is nonzero, all `k` registers are nonzero. + pub fn lower_bound( + &self, + lg_config_k: u8, + cur_min: u8, + num_at_cur_min: u32, + num_std_dev: NumStdDev, + ) -> f64 { + let estimate = self.estimate(lg_config_k, cur_min, num_at_cur_min); + let rse = relative_error( + lg_config_k, + false, + self.uses_composite_estimate(), + num_std_dev, + ); + let config_k = 1u32 << lg_config_k; + let num_nonzero_registers = if cur_min == 0 { + config_k - num_at_cur_min + } else { + config_k + }; + (estimate / (1.0 + rse)).max(f64::from(num_nonzero_registers)) } fn raw_estimate(&self, lg_config_k: u8) -> f64 { @@ -128,13 +164,13 @@ impl RegisterStatistics { _ => 0.7213 / (1.0 + 1.079 / k), }; - (correction_factor * k * k) / (self.kxq.low + self.kxq.high) + (correction_factor * k * k) / (self.kxq0 + self.kxq1) } - fn bitmap_estimate(&self, lg_config_k: u8) -> f64 { + fn bitmap_estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { let k = 1 << lg_config_k; - let num_unhit = self.num_zeros; + let num_unhit = if cur_min == 0 { num_at_cur_min } else { 0 }; // Preserve the upstream fallback for a bitmap with no empty registers. if num_unhit == 0 { @@ -146,7 +182,7 @@ impl RegisterStatistics { } /// Selects between bias-corrected HLL and bitmap estimates using their empirical crossover. - pub fn composite_estimate(&self, lg_config_k: u8) -> f64 { + pub fn composite_estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { let raw_estimate = self.raw_estimate(lg_config_k); let x_arr = composite_interpolation::get_x_arr(lg_config_k); @@ -176,7 +212,7 @@ impl RegisterStatistics { return adjusted_estimate; } - let linear_estimate = self.bitmap_estimate(lg_config_k); + let linear_estimate = self.bitmap_estimate(lg_config_k, cur_min, num_at_cur_min); // Comparing the average to the threshold reduces estimator-selection bias. let average_estimate = (adjusted_estimate + linear_estimate) / 2.0; @@ -196,12 +232,51 @@ impl RegisterStatistics { linear_estimate } } + + pub fn hip_accum(&self) -> f64 { + match self.estimate_state { + EstimateState::Hip(hip_accum) => hip_accum, + EstimateState::Composite => 0.0, + } + } + + pub fn kxq0(&self) -> f64 { + self.kxq0 + } + + pub fn kxq1(&self) -> f64 { + self.kxq1 + } + + pub fn uses_composite_estimate(&self) -> bool { + matches!(self.estimate_state, EstimateState::Composite) + } + + pub fn estimate_state(&self) -> EstimateState { + self.estimate_state + } + + /// Restores estimate state after copying or transforming the same logical sketch. + pub fn restore_estimate_state(&mut self, state: EstimateState) { + self.estimate_state = state; + } + + /// Invalidates HIP after registers from independent histories are merged. + pub fn invalidate_hip(&mut self) { + self.estimate_state = EstimateState::Composite; + } + + /// Replaces register-derived KxQ values after a bulk register operation. + pub fn restore_kxq(&mut self, kxq0: f64, kxq1: f64) { + self.kxq0 = kxq0; + self.kxq1 = kxq1; + } } /// Relative error used as `estimate / (1 + error)`: negative for upper bounds. /// Follows datasketches-cpp's `HllUtil::getRelErr`: /// -pub fn relative_error( +fn relative_error( lg_config_k: u8, upper_bound: bool, composite: bool, @@ -364,3 +439,101 @@ static NON_HIP_UB: [f64; 27] = [ -0.031827816, -0.046973459, // lg_k=12 ]; + +#[cfg(test)] +mod tests { + use googletest::assert_that; + use googletest::prelude::gt; + use googletest::prelude::lt; + + use super::*; + + #[test] + fn lower_bound_is_clamped_to_the_non_zero_register_count() { + let lg_config_k = 4; + let mut estimator = Estimator::new(lg_config_k); + for _ in 0..8 { + estimator.update(lg_config_k, 0, 1); + } + + assert_eq!( + estimator.lower_bound(lg_config_k, 0, 8, NumStdDev::Three), + 8.0 + ); + } + + #[test] + fn lower_bound_is_clamped_to_config_k_when_every_register_is_hit() { + let lg_config_k = 4; + let mut estimator = Estimator::new(lg_config_k); + for _ in 0..16 { + estimator.update(lg_config_k, 0, 1); + } + + assert_eq!( + estimator.lower_bound(lg_config_k, 1, 16, NumStdDev::Three), + 16.0 + ); + } + + #[test] + fn test_estimator_initialization() { + let est = Estimator::new(10); // 1024 registers + + assert_eq!(est.hip_accum(), 0.0); + assert_eq!(est.kxq0(), 1024.0); // All zeros = 1.0 each + assert_eq!(est.kxq1(), 0.0); + assert!(!est.uses_composite_estimate()); + } + + #[test] + fn test_estimator_update() { + let mut est = Estimator::new(8); // 256 registers + + est.update(8, 0, 10); + + assert_that!(est.hip_accum(), gt(0.0)); + + assert_that!(est.kxq0(), lt(256.0)); + assert_eq!(est.kxq1(), 0.0); + } + + #[test] + fn test_kxq_split() { + let mut est = Estimator::new(8); + + est.update(8, 0, 10); + let kxq0_after_10 = est.kxq0(); + let kxq1_after_10 = est.kxq1(); + + assert_that!(kxq0_after_10, lt(256.0)); + assert_eq!(kxq1_after_10, 0.0); + + // Update from 10 to 50 (crosses the 32 boundary) + est.update(8, 10, 50); + let kxq0_after_50 = est.kxq0(); + let kxq1_after_50 = est.kxq1(); + + assert_that!(kxq0_after_50, lt(kxq0_after_10)); // Removed 1/2^10 from kxq0 (decreases kxq0) + assert_that!(kxq1_after_50, gt(0.0)); // Added 1/2^50 to kxq1 + } + + #[test] + fn test_invalidate_hip() { + let mut est = Estimator::new(10); + + est.update(10, 0, 5); + let hip_normal = est.hip_accum(); + assert_that!(hip_normal, gt(0.0)); + + est.invalidate_hip(); + assert!(est.uses_composite_estimate()); + assert_eq!(est.hip_accum(), 0.0); + + // Register-derived state continues to update after HIP is invalidated. + let kxq0_before = est.kxq0(); + est.update(10, 5, 10); + assert_eq!(est.hip_accum(), 0.0); + assert_ne!(est.kxq0(), kxq0_before); + } +} diff --git a/datasketches/src/hll/mod.rs b/datasketches/src/hll/mod.rs index 0c08fa4d..06db8d35 100644 --- a/datasketches/src/hll/mod.rs +++ b/datasketches/src/hll/mod.rs @@ -61,7 +61,6 @@ use std::hash::Hash; use crate::hash::MurmurHash3X64128; -mod array; mod array4; mod array6; mod array8; diff --git a/datasketches/src/hll/sketch.rs b/datasketches/src/hll/sketch.rs index 70282e17..910a730c 100644 --- a/datasketches/src/hll/sketch.rs +++ b/datasketches/src/hll/sketch.rs @@ -464,9 +464,9 @@ impl HllSketch { match &self.mode { Mode::List { list, hll_type } => list.serialize(self.lg_config_k, *hll_type), Mode::Set { set, hll_type } => set.serialize(self.lg_config_k, *hll_type), - Mode::Array4(arr) => arr.serialize(), - Mode::Array6(arr) => arr.serialize(), - Mode::Array8(arr) => arr.serialize(), + Mode::Array4(arr) => arr.serialize(self.lg_config_k), + Mode::Array6(arr) => arr.serialize(self.lg_config_k), + Mode::Array8(arr) => arr.serialize(self.lg_config_k), } } diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index a4e38e65..cef8bf9f 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -415,12 +415,6 @@ fn convert_coupon_mode_to_hll8(src_mode: &Mode, src_lg_k: u8) -> HllSketch { } fn merge_coupons_into_gadget(gadget: &mut HllSketch, src_mode: &Mode) { - // Dense coupon updates cannot change representation, so dispatch once for the batch. - if let Mode::Array8(dst) = gadget.mode_mut() { - merge_coupons_into_mode(dst, src_mode); - return; - } - match src_mode { Mode::List { list, .. } => { for coupon in list.container().iter() { @@ -490,7 +484,14 @@ fn get_array_estimate_state(mode: &Mode) -> EstimateState { } fn merge_array46_same_lgk(dst: &mut Array8, num_registers: usize, get_value: impl Fn(u32) -> u8) { - dst.merge_registers((0..num_registers).map(|slot| (slot, get_value(slot as u32)))); + for slot in 0..num_registers { + let val = get_value(slot as u32); + let current = dst.values()[slot]; + if val > current { + dst.set_register(slot, val); + } + } + dst.invalidate_estimator(); } /// Merging independent register histories invalidates the HIP accumulator. @@ -517,12 +518,18 @@ fn merge_array46_with_downsample( num_registers: usize, get_value: impl Fn(u32) -> u8, ) { - let mask = (1 << dst_lg_k) - 1; - dst.merge_registers( - (0..num_registers) - .map(|slot| (slot & mask, get_value(slot as u32))) - .filter(|&(_, value)| value > 0), - ); + let dst_mask = (1 << dst_lg_k) - 1; + for src_slot in 0..num_registers { + let val = get_value(src_slot as u32); + if val > 0 { + let dst_slot = (src_slot as u32 & dst_mask) as usize; + let current = dst.values()[dst_slot]; + if val > current { + dst.set_register(dst_slot, val); + } + } + } + dst.invalidate_estimator(); } /// Multiple source registers map to each destination register via masking. diff --git a/tests-integration/tests/hll_test/estimate.rs b/tests-integration/tests/hll_test/estimate.rs index ef0f5e8d..37242598 100644 --- a/tests-integration/tests/hll_test/estimate.rs +++ b/tests-integration/tests/hll_test/estimate.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use datasketches::common::NumStdDev; use datasketches::hll::HllSketch; use datasketches::hll::HllType; use datasketches::hll::HllUnion; @@ -24,44 +23,6 @@ use googletest::prelude::near; const HLL_TYPES: [HllType; 3] = [HllType::Hll4, HllType::Hll6, HllType::Hll8]; -#[test] -fn estimates_survive_minimum_shifts_and_round_trips() { - let mut sketches = HLL_TYPES.map(|hll_type| HllSketch::new(4, hll_type).unwrap()); - let mut start = 0; - // At this precision, the growing stream fills every register and repeatedly raises - // HLL4's packing minimum. Its minimum count must not be treated as a zero count. - for end in [64, 512, 4_096] { - for value in start..end { - for sketch in &mut sketches { - sketch.update(value as u64); - } - } - let expected = &sketches[2]; - for sketch in &sketches { - assert_eq!(sketch.estimate(), expected.estimate()); - assert_eq!(sketch.composite_estimate(), expected.composite_estimate()); - for num_std_dev in [NumStdDev::One, NumStdDev::Two, NumStdDev::Three] { - assert_eq!( - sketch.lower_bound(num_std_dev), - expected.lower_bound(num_std_dev) - ); - assert_eq!( - sketch.upper_bound(num_std_dev), - expected.upper_bound(num_std_dev) - ); - } - } - for sketch in &mut sketches { - *sketch = HllSketch::deserialize(&sketch.serialize()).unwrap(); - } - start = end; - } - assert_eq!(sketches[0].estimate(), sketches[2].estimate()); - assert_eq!( - sketches[0].composite_estimate(), - sketches[2].composite_estimate() - ); -} fn make_sketch(hll_type: HllType, values: impl Iterator) -> HllSketch { let mut sketch = HllSketch::new(11, hll_type).unwrap(); for value in values { From b2fcd5cd7c9a64daaf7a4728c5465e5e34f6bb60 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 11:19:57 +0800 Subject: [PATCH 8/9] refactor(hll): clarify deferred estimation names Name the HLL8 zero-count and estimator bundle EstimationData and its OnceLock field estimation. Align the accessor, reconstruction helper, and local variables with the estimation data they provide. Use state for Estimator's EstimateState field. These naming changes retain the Array8-local deferred reconstruction and direct estimator maintenance in HLL4 and HLL6. Validation: cargo x check, cargo x test, cargo x lint, and Rust 1.86 HLL tests. --- datasketches/src/hll/array8.rs | 112 ++++++++++++++++-------------- datasketches/src/hll/estimator.rs | 26 +++---- 2 files changed, 70 insertions(+), 68 deletions(-) diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index c111b025..0fa5a516 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -39,13 +39,14 @@ use crate::hll::serialization::encode_mode_byte; pub struct Array8 { lg_config_k: u8, bytes: Box<[u8]>, - /// An absent cache means a bulk merge invalidated HIP and the register-derived fields. + /// Absent estimation data means a bulk merge invalidated HIP and the register-derived fields. /// Shared queries initialize it once; exclusive updates can maintain or invalidate it. - cache: OnceLock, + estimation: OnceLock, } +/// Zero-register count and estimator used for cardinality queries. #[derive(Debug, Clone, PartialEq)] -struct CachedState { +struct EstimationData { num_zeros: u32, estimator: Estimator, } @@ -54,7 +55,7 @@ impl PartialEq for Array8 { fn eq(&self, other: &Self) -> bool { self.lg_config_k == other.lg_config_k && self.bytes == other.bytes - && self.cached_state() == other.cached_state() + && self.estimation() == other.estimation() } } @@ -65,7 +66,7 @@ impl Array8 { Self { lg_config_k, bytes: vec![0u8; k as usize].into_boxed_slice(), - cache: OnceLock::from(CachedState { + estimation: OnceLock::from(EstimationData { num_zeros: k, estimator: Estimator::new(lg_config_k), }), @@ -90,12 +91,12 @@ impl Array8 { let old_value = self.get(slot); if new_value > old_value { - if let Some(cache) = self.cache.get_mut() { - cache + if let Some(estimation) = self.estimation.get_mut() { + estimation .estimator .update(self.lg_config_k, old_value, new_value); if old_value == 0 { - cache.num_zeros -= 1; + estimation.num_zeros -= 1; } } self.put(slot, new_value); @@ -103,37 +104,37 @@ impl Array8 { } pub fn estimate(&self) -> f64 { - let cache = self.cached_state(); + let estimation = self.estimation(); // Array8 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros - cache + estimation .estimator - .estimate(self.lg_config_k, 0, cache.num_zeros) + .estimate(self.lg_config_k, 0, estimation.num_zeros) } pub fn composite_estimate(&self) -> f64 { - let cache = self.cached_state(); - cache + let estimation = self.estimation(); + estimation .estimator - .composite_estimate(self.lg_config_k, 0, cache.num_zeros) + .composite_estimate(self.lg_config_k, 0, estimation.num_zeros) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - let cache = self.cached_state(); - cache + let estimation = self.estimation(); + estimation .estimator - .upper_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + .upper_bound(self.lg_config_k, 0, estimation.num_zeros, num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - let cache = self.cached_state(); - cache + let estimation = self.estimation(); + estimation .estimator - .lower_bound(self.lg_config_k, 0, cache.num_zeros, num_std_dev) + .lower_bound(self.lg_config_k, 0, estimation.num_zeros, num_std_dev) } pub fn is_empty(&self) -> bool { - if let Some(cache) = self.cache.get() { - return cache.num_zeros == (1 << self.lg_config_k); + if let Some(estimation) = self.estimation.get() { + return estimation.num_zeros == (1 << self.lg_config_k); } // Union checks emptiness between merges. A nonzero register answers that without // rebuilding all estimator fields and defeating deferred reconstruction. @@ -149,18 +150,20 @@ impl Array8 { } pub fn estimate_state(&self) -> EstimateState { - self.cache.get().map_or(EstimateState::Composite, |cache| { - cache.estimator.estimate_state() - }) + self.estimation + .get() + .map_or(EstimateState::Composite, |estimation| { + estimation.estimator.estimate_state() + }) } pub fn restore_estimate_state(&mut self, state: EstimateState) { - if let Some(cache) = self.cache.get_mut() { - cache.estimator.restore_estimate_state(state); + if let Some(estimation) = self.estimation.get_mut() { + estimation.estimator.restore_estimate_state(state); } else if let EstimateState::Hip(_) = state { - let mut cache = self.rebuild_cached_values(); - cache.estimator.restore_estimate_state(state); - self.cache = OnceLock::from(cache); + let mut estimation = self.rebuild_estimation(); + estimation.estimator.restore_estimate_state(state); + self.estimation = OnceLock::from(estimation); } } @@ -171,7 +174,7 @@ impl Array8 { /// Invalidates HIP and defers rebuilding register-derived fields until a query needs them. pub fn invalidate_estimator(&mut self) { - self.cache.take(); + self.estimation.take(); } /// Merges registers and invalidates HIP because their update order is unavailable. @@ -212,11 +215,11 @@ impl Array8 { self.invalidate_estimator(); } - fn cached_state(&self) -> &CachedState { - self.cache.get_or_init(|| self.rebuild_cached_values()) + fn estimation(&self) -> &EstimationData { + self.estimation.get_or_init(|| self.rebuild_estimation()) } - fn rebuild_cached_values(&self) -> CachedState { + fn rebuild_estimation(&self) -> EstimationData { let num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; let mut kxq0_sum = 0.0; @@ -235,7 +238,7 @@ impl Array8 { let mut estimator = Estimator::new(self.lg_config_k); estimator.restore_kxq(kxq0_sum, kxq1_sum); estimator.invalidate_hip(); - CachedState { + EstimationData { num_zeros, estimator, } @@ -284,7 +287,7 @@ impl Array8 { Ok(Self { lg_config_k, bytes: data.into_boxed_slice(), - cache: OnceLock::from(CachedState { + estimation: OnceLock::from(EstimationData { num_zeros, estimator, }), @@ -292,7 +295,7 @@ impl Array8 { } pub fn serialize(&self, lg_config_k: u8) -> Vec { - let cache = self.cached_state(); + let estimation = self.estimation(); let k = 1 << lg_config_k; let total_size = HLL_PREAMBLE_SIZE + k as usize; let mut bytes = SketchBytes::with_capacity(total_size); @@ -304,7 +307,7 @@ impl Array8 { bytes.write_u8(0); // unused for HLL mode let mut flags = 0u8; - if cache.estimator.uses_composite_estimate() { + if estimation.estimator.uses_composite_estimate() { flags |= OUT_OF_ORDER_FLAG_MASK; } bytes.write_u8(flags); @@ -314,12 +317,12 @@ impl Array8 { bytes.write_u8(encode_mode_byte(CUR_MODE_HLL, TGT_HLL8)); - bytes.write_f64_le(cache.estimator.hip_accum()); - bytes.write_f64_le(cache.estimator.kxq0()); - bytes.write_f64_le(cache.estimator.kxq1()); + bytes.write_f64_le(estimation.estimator.hip_accum()); + bytes.write_f64_le(estimation.estimator.kxq0()); + bytes.write_f64_le(estimation.estimator.kxq1()); // Write num_at_cur_min (num_zeros for Array8) - bytes.write_u32_le(cache.num_zeros); + bytes.write_u32_le(estimation.num_zeros); // Write aux_count (always 0 for Array8) bytes.write_u32_le(0); @@ -449,11 +452,11 @@ mod tests { arr.update(Coupon::pack(1, 50)); // value >= 32, goes to kxq1 // Initial kxq0 = 256 (all zeros = 1.0 each) - assert_that!(arr.cached_state().estimator.kxq0(), lt(256.0)); + assert_that!(arr.estimation().estimator.kxq0(), lt(256.0)); // kxq1 should have a positive value (from 1/2^50) - assert_that!(arr.cached_state().estimator.kxq1(), gt(0.0)); - assert_that!(arr.cached_state().estimator.kxq1(), lt(1e-10)); + assert_that!(arr.estimation().estimator.kxq1(), gt(0.0)); + assert_that!(arr.estimation().estimator.kxq1(), lt(1e-10)); } #[test] @@ -493,9 +496,9 @@ mod tests { assert_eq!(dst.get(3), 40, "dst[3] got new value"); // Bulk merges require composite estimation. - assert!(dst.cached_state().estimator.uses_composite_estimate()); + assert!(dst.estimation().estimator.uses_composite_estimate()); - assert_eq!(dst.cached_state().num_zeros, 12); + assert_eq!(dst.estimation().num_zeros, 12); } #[test] @@ -519,7 +522,7 @@ mod tests { assert_eq!(dst.get(1), 30, "dst[1] = max(20, 18, 30)"); // Bulk merges require composite estimation. - assert!(dst.cached_state().estimator.uses_composite_estimate()); + assert!(dst.estimation().estimator.uses_composite_estimate()); } #[test] @@ -541,7 +544,7 @@ mod tests { } #[test] - fn test_rebuild_cached_values() { + fn test_rebuild_estimation() { let mut arr = Array8::new(4); // 16 slots arr.put(0, 10); @@ -553,13 +556,16 @@ mod tests { arr.put(5, 63); arr.invalidate_estimator(); - let cache = arr.cached_state(); - assert_eq!(cache.num_zeros, 10); + let estimation = arr.estimation(); + assert_eq!(estimation.num_zeros, 10); assert_eq!( - cache.estimator.kxq0(), + estimation.estimator.kxq0(), 10.0 + 2_f64.powi(-10) + 2_f64.powi(-20) + 2_f64.powi(-30) + 2_f64.powi(-31) ); - assert_eq!(cache.estimator.kxq1(), 2_f64.powi(-32) + 2_f64.powi(-63)); + assert_eq!( + estimation.estimator.kxq1(), + 2_f64.powi(-32) + 2_f64.powi(-63) + ); } #[test] diff --git a/datasketches/src/hll/estimator.rs b/datasketches/src/hll/estimator.rs index a72fcca0..631d31ff 100644 --- a/datasketches/src/hll/estimator.rs +++ b/datasketches/src/hll/estimator.rs @@ -42,7 +42,7 @@ pub enum EstimateState { /// precision when large and small contributions coexist. #[derive(Debug, Clone, PartialEq)] pub struct Estimator { - estimate_state: EstimateState, + state: EstimateState, /// KxQ register for values < 32 (larger inverse powers) kxq0: f64, /// KxQ register for values >= 32 (tiny inverse powers) @@ -53,7 +53,7 @@ impl Estimator { pub fn new(lg_config_k: u8) -> Self { let k = 1 << lg_config_k; Self { - estimate_state: EstimateState::Hip(0.0), + state: EstimateState::Hip(0.0), kxq0: k as f64, // All registers start at 0, so kxq0 = k * (1/2^0) = k kxq1: 0.0, } @@ -61,16 +61,12 @@ impl Estimator { /// Restores estimator fields from an HLL serialization preamble. pub fn from_serialized(hip_accum: f64, kxq0: f64, kxq1: f64, out_of_order: bool) -> Self { - let estimate_state = if out_of_order { + let state = if out_of_order { EstimateState::Composite } else { EstimateState::Hip(hip_accum) }; - Self { - estimate_state, - kxq0, - kxq1, - } + Self { state, kxq0, kxq1 } } /// Applies a register increase; requires `new_value > old_value`. @@ -78,7 +74,7 @@ impl Estimator { let k = (1 << lg_config_k) as f64; // HIP must use the KxQ sums from before this register update. - if let EstimateState::Hip(hip_accum) = &mut self.estimate_state { + if let EstimateState::Hip(hip_accum) = &mut self.state { *hip_accum += k / (self.kxq0 + self.kxq1); } @@ -103,7 +99,7 @@ impl Estimator { /// For `Array6`/`Array8`, pass zero for `cur_min` and the zero-register count for /// `num_at_cur_min`. `Array4` tracks its actual minimum and its count. pub fn estimate(&self, lg_config_k: u8, cur_min: u8, num_at_cur_min: u32) -> f64 { - match self.estimate_state { + match self.state { EstimateState::Hip(hip_accum) => hip_accum, EstimateState::Composite => { self.composite_estimate(lg_config_k, cur_min, num_at_cur_min) @@ -234,7 +230,7 @@ impl Estimator { } pub fn hip_accum(&self) -> f64 { - match self.estimate_state { + match self.state { EstimateState::Hip(hip_accum) => hip_accum, EstimateState::Composite => 0.0, } @@ -249,21 +245,21 @@ impl Estimator { } pub fn uses_composite_estimate(&self) -> bool { - matches!(self.estimate_state, EstimateState::Composite) + matches!(self.state, EstimateState::Composite) } pub fn estimate_state(&self) -> EstimateState { - self.estimate_state + self.state } /// Restores estimate state after copying or transforming the same logical sketch. pub fn restore_estimate_state(&mut self, state: EstimateState) { - self.estimate_state = state; + self.state = state; } /// Invalidates HIP after registers from independent histories are merged. pub fn invalidate_hip(&mut self) { - self.estimate_state = EstimateState::Composite; + self.state = EstimateState::Composite; } /// Replaces register-derived KxQ values after a bulk register operation. From 7905e4bd071b473ec0575a0440d6bf88dbba4ccd Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 4 Nov 2026 11:34:20 +0800 Subject: [PATCH 9/9] perf(hll): avoid cloning registers when reducing union precision When a dense input lowers a union's effective precision, the existing HLL8 gadget was cloned into a temporary Mode solely to dispatch the register downsampling operation. Borrow its register slice and call the Array8 operation directly, removing a full register allocation and copy. The operation still invalidates HIP and defers rebuilding estimation fields until they are needed. Rename EstimationData to Estimation and destructure its fields at query sites to make access to the estimator and zero-register count clearer. Validation: cargo x check, cargo x test, and cargo x lint. --- datasketches/src/hll/array8.rs | 50 ++++++++++++++++++---------------- datasketches/src/hll/union.rs | 11 ++------ 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/datasketches/src/hll/array8.rs b/datasketches/src/hll/array8.rs index 0fa5a516..65861044 100644 --- a/datasketches/src/hll/array8.rs +++ b/datasketches/src/hll/array8.rs @@ -41,12 +41,12 @@ pub struct Array8 { bytes: Box<[u8]>, /// Absent estimation data means a bulk merge invalidated HIP and the register-derived fields. /// Shared queries initialize it once; exclusive updates can maintain or invalidate it. - estimation: OnceLock, + estimation: OnceLock, } /// Zero-register count and estimator used for cardinality queries. #[derive(Debug, Clone, PartialEq)] -struct EstimationData { +struct Estimation { num_zeros: u32, estimator: Estimator, } @@ -66,7 +66,7 @@ impl Array8 { Self { lg_config_k, bytes: vec![0u8; k as usize].into_boxed_slice(), - estimation: OnceLock::from(EstimationData { + estimation: OnceLock::from(Estimation { num_zeros: k, estimator: Estimator::new(lg_config_k), }), @@ -104,32 +104,36 @@ impl Array8 { } pub fn estimate(&self) -> f64 { - let estimation = self.estimation(); + let Estimation { + num_zeros, + estimator, + } = self.estimation(); // Array8 doesn't use cur_min (always 0), so num_at_cur_min = num_zeros - estimation - .estimator - .estimate(self.lg_config_k, 0, estimation.num_zeros) + estimator.estimate(self.lg_config_k, 0, *num_zeros) } pub fn composite_estimate(&self) -> f64 { - let estimation = self.estimation(); - estimation - .estimator - .composite_estimate(self.lg_config_k, 0, estimation.num_zeros) + let Estimation { + num_zeros, + estimator, + } = self.estimation(); + estimator.composite_estimate(self.lg_config_k, 0, *num_zeros) } pub fn upper_bound(&self, num_std_dev: NumStdDev) -> f64 { - let estimation = self.estimation(); - estimation - .estimator - .upper_bound(self.lg_config_k, 0, estimation.num_zeros, num_std_dev) + let Estimation { + num_zeros, + estimator, + } = self.estimation(); + estimator.upper_bound(self.lg_config_k, 0, *num_zeros, num_std_dev) } pub fn lower_bound(&self, num_std_dev: NumStdDev) -> f64 { - let estimation = self.estimation(); - estimation - .estimator - .lower_bound(self.lg_config_k, 0, estimation.num_zeros, num_std_dev) + let Estimation { + num_zeros, + estimator, + } = self.estimation(); + estimator.lower_bound(self.lg_config_k, 0, *num_zeros, num_std_dev) } pub fn is_empty(&self) -> bool { @@ -215,11 +219,11 @@ impl Array8 { self.invalidate_estimator(); } - fn estimation(&self) -> &EstimationData { + fn estimation(&self) -> &Estimation { self.estimation.get_or_init(|| self.rebuild_estimation()) } - fn rebuild_estimation(&self) -> EstimationData { + fn rebuild_estimation(&self) -> Estimation { let num_zeros = self.bytes.iter().filter(|&&v| v == 0).count() as u32; let mut kxq0_sum = 0.0; @@ -238,7 +242,7 @@ impl Array8 { let mut estimator = Estimator::new(self.lg_config_k); estimator.restore_kxq(kxq0_sum, kxq1_sum); estimator.invalidate_hip(); - EstimationData { + Estimation { num_zeros, estimator, } @@ -287,7 +291,7 @@ impl Array8 { Ok(Self { lg_config_k, bytes: data.into_boxed_slice(), - estimation: OnceLock::from(EstimationData { + estimation: OnceLock::from(Estimation { num_zeros, estimator, }), diff --git a/datasketches/src/hll/union.rs b/datasketches/src/hll/union.rs index cef8bf9f..f942fc00 100644 --- a/datasketches/src/hll/union.rs +++ b/datasketches/src/hll/union.rs @@ -166,15 +166,10 @@ impl HllUnion { match self.gadget.mode() { Mode::Array8(old_gadget) => { - merge_array_with_downsample( - &mut new_array, - src_lg_k, - &Mode::Array8(old_gadget.clone()), - dst_lg_k, - ); + new_array.merge_array_with_downsample(old_gadget.values(), dst_lg_k); } _ => { - unreachable!("gadget mode changed unexpectedly; should never be Array4/Array6") + unreachable!("gadget mode changed unexpectedly; should never be Array4/Array6"); } } @@ -186,7 +181,7 @@ impl HllUnion { merge_array_into_array8(dst_array, dst_lg_k, src_mode, src_lg_k); } _ => { - unreachable!("gadget mode changed unexpectedly; should never be Array4/Array6") + unreachable!("gadget mode changed unexpectedly; should never be Array4/Array6"); } } }