tisonkun commented on PR #7345: URL: https://github.com/apache/datafusion-comet/pull/7345#issuecomment-6538732315
I've opened an upstream PR for the DataSketches Rust costs identified here and in #7341: https://github.com/apache/datasketches-rust/pull/317. It contains three separate commits: - Copy dense HLL8 inputs, including their estimator state, without rescanning the registers when no downsampling is needed. - Add `HllUnion::serialize()` to emit HLL8 sketch bytes directly, avoiding `to_sketch(HllType::Hll8).serialize()` and its intermediate copy. - Defer register-cache reconstruction across dense merges until an estimate, bound, or serialized result is needed, while retaining shared-reference queries and thread safety. In the library benchmarks at lg_k 18, merging 16 dense partials and then querying drops from 4.91 ms to 0.284 ms; recreating a union for each of 400 small partials drops from 79.71 ms to 3.10 ms. These are library measurements, not end-to-end Comet results. Keeping a persistent union as this PR does remains useful. Reviews and feedback from the Comet workload would be very welcome, and hopefully Comet can pick these up in a subsequent upgrade once they are merged and released. Newer releases already provide `estimated_size()` and fixes for compact HLL decoding and single-input HIP preservation, which can also remove several 0.3.0 workarounds. Using `estimated_size()` instead of calling `estimate()` for memory accounting will help preserve the benefit of deferred reconstruction. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
