Skip to content

perf(tuple): serialize union results without cloning summaries - #322

Open
Aschenbath wants to merge 2 commits into
apache:mainfrom
Aschenbath:tuple-union-serialize
Open

Aschenbath wants to merge 2 commits into
apache:mainfrom
Aschenbath:tuple-union-serialize

Conversation

@Aschenbath

Copy link
Copy Markdown

Closes #321.

What

  • Adds TupleUnion::serialize(&self, ordered: bool) -> Vec<u8>. It needs P::Summary: TupleSummaryValue but not Clone.
  • UnionState has one result-preparation routine (theta filtering, trimming to nominal capacity, ordering), and it takes a projection. to_compact_sketch_state projects with E::clone, so the Theta path compiles to the same code as before. The new to_borrowed_compact_sketch_state (gated on tuple) projects to &E. SketchEntry is implemented for &E to support this.
  • The compact Tuple encoder moved into serialize_compact_state, which is generic over E: Borrow<TupleEntry<S>>. CompactTupleSketch::serialize and TupleUnion::serialize both use it, so the format logic exists only once.

When ordered is true, the output is byte-identical to union.to_sketch(true).serialize(). For unordered results with trimming, entries can come out in a different order than in the cloning path, because select_nth_unstable may pick a different partition scheme depending on the element size. The retained entries, theta, seed hash and flags are the same. The doc comment states this.

I started with TupleUnion only, as the issue suggests. TupleIntersection and mutable-sketch compaction aren't touched.

Tests

Added to tests-integration/tests/serde_tests/tuple.rs:

  • union_serialization_matches_compact_result (u64 summaries) compares serialize with to_sketch(..).serialize() for both orderings. The cases are: a fresh union, an empty input, a single entry with combined summaries, exact mode (30 entries), trimming to nominal capacity (lg_k 5 → 32 entries), a sampled union (p = 0.5), and a non-empty result with every entry screened by theta. Ordered results must match byte for byte. Unordered results must have the same entries, theta, seed hash and flags.
  • union_serialization_with_heap_backed_summaries_supports_later_merges uses a variable-size Vec<String> summary with a custom codec, in estimation mode. It round-trips the result, merges the restored sketch with another sketch, and compares against merging to_sketch() directly.

cargo x test, cargo +nightly clippy --tests --all-features --all-targets --workspace -- -D warnings, cargo +nightly fmt --all --check and typos pass, and cargo +1.86.0 check -p datasketches --all-features (MSRV) passes too. I ran these on Windows x86_64. I didn't run taplo or hawkeye locally: no TOML changed, and the new benchmark file has the license header.

Benchmarks

New benchmarks/tuple/serde.rs compares serialize with to_sketch(..).serialize(). One input sketch goes into a union (default lg_k 12). Medians from Divan with AllocProfiler, unordered (ordered is similar):

Heap-backed summary (4 short Strings per key):

rows to_sketch().serialize() serialize() allocations
4 954 ns 252 ns 22 → 2
100 23.5 µs 3.8 µs 502 → 2
80,000 2.20 ms 0.25 ms 31,757 → 2

u64 summary, from two runs with 500 samples:

rows to_sketch().serialize() serialize()
4 105–113 ns 138–140 ns
100 0.81–0.90 µs 1.08–1.11 µs
80,000 26–29 µs 29–38 µs

So the gain is for summaries that are expensive to clone. For small Copy summaries the borrowed path is about 15–25% slower at small sizes, a few hundred nanoseconds. Cloning 16-byte entries into a contiguous vector is about as cheap as collecting pointers, and the encoder then has to dereference each pointer. The CHANGELOG entry only claims the heap-backed case. If you'd rather keep u64 users on the cloning path, I can document that on serialize. Another option is to collect (hash, &summary) pairs so that sorting doesn't dereference.


This PR was written by an AI agent (Claude Code): the code, tests, benchmarks and this description. The verification above is what the agent ran.

🤖 Generated with Claude Code

Add TupleUnion::serialize. It prepares the union result over borrowed
entries and encodes the summaries in place, so serializing no longer
clones every retained summary into an intermediate CompactTupleSketch,
and the summary type does not need to implement Clone.

The union result preparation (theta filtering, trimming to nominal
capacity, ordering) is shared between the cloning and the borrowing
paths, and CompactTupleSketch::serialize uses the same encoder.

Closes apache#321

Generated-by: Claude Code (Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

I have a few comments:

  1. compact_sketch_state_with looks over abstraction.
  2. Perhaps you can do the same for ThetaUnion and we may see some shared logics or common pattern for peer review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(tuple): serialize union results without cloning summaries

2 participants