feat(tdigest): add owned batch construction and quantile APIs - #261
Open
tisonkun wants to merge 6 commits into
Open
feat(tdigest): add owned batch construction and quantile APIs#261tisonkun wants to merge 6 commits into
tisonkun wants to merge 6 commits into
Conversation
tisonkun
marked this pull request as draft
September 1, 2026 23:31
tisonkun
marked this pull request as ready for review
September 2, 2026 03:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FromIterator<TDigestMut>so owned partial digests can be combined with one compression passTDigestMut::merge(&TDigestMut)as the single borrowed merge API, while using the smallerkwhen inputs differTDigestMut::quantilesandTDigest::quantiles; nondecreasing ranks share one centroid scan, while arbitrary input order is preservedDeserialization invariant validation landed independently in #262 and is now part of the base branch rather than this PR's diff.
Ownership and clone cost
The existing borrowed
mergedoes not clone the complete right-hand digest. It moves the receiver's buffer into the result and copies the right-hand centroids into the new contiguous result buffer. Requiring an owned argument when the caller must retain it would add a whole-buffer allocation and copy before that merge work.A temporary clone-cost probe on the same machine measured:
The public APIs therefore cover the two ownership cases without forcing a clone:
merge(&other)when the source must remain availablepartials.into_iter().collect::<TDigestMut>()when the partials can be consumedCollection ignores empty inputs, returns a single non-empty input unchanged, and uses the smallest
kamong non-empty inputs.FromIteratoris infallible, matching the existingupdateandmergeAPIs; representation overflow is documented as a panic.Implementation shape
Batch construction separates cardinality and storage cases:
TDigestMut::default(); one non-empty input is returned unchangedkwithout mutating the resultFor batch queries,
QuantileCursorowns the monotonic scan state. Already-sorted ranks use it directly. Arbitrary ranks sort indices rather than values, drive the same cursor in rank order, and place answers back in the caller's original order.The separate deserialization validation in #262 establishes that fully compressed buffers are sorted. Pairwise merge now treats that as an internal invariant, retaining
debug_assertchecks without rescanning both buffers in release builds.Memory behavior
One-pass batch construction must retain the input centroids until the global merge/compression step. It therefore trades higher peak live input memory for less recompression. Callers that need bounded additional memory can continue to deserialize and call borrowed
mergeone state at a time, or merge bounded chunks.Performance
Representative local Divan medians after merging the current
main:mergecollectquantilequantilesThe owned benchmark creates fresh inputs with Divan
with_inputs, so cloning test fixtures is outside the measured interval. The measured interval does include releasing the 64 consumed source buffers. The owned path performs 2 allocations (131 KB) versus 53 allocations (234.5 KB) for repeated merge.Validation
cargo x prepare-testdatacargo x checkcargo x lintcargo x testcargo bench --package benchmarks --bench benchmarks -- tdigest::merge::partials --sample-count 300cargo bench --package benchmarks --bench benchmarks -- tdigest::query::quantiles --sample-count 500