Repository navigation
refactor(tdigest): organize public APIs and internal helpers - #313
Merged
Merged
Conversation
tisonkun
enabled auto-merge (squash)
October 27, 2026 15:17
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.
Public operations, storage management, query helpers, and serialization are interleaved in
sketch.rs, making the implementation difficult to follow. Group the public APIs first, keep complete algorithms together, and name query helpers around interpolation intervals and cumulative weights. Digest-independent interpolation becomes a module-level function; construction remains associated with the type.Keep codec implementation and format constants together at the end of the same module to preserve its compilation boundary. Public APIs, query arithmetic, validation, and serialization formats are unchanged.
Performance methodology
Compared baseline
8bb74d7fe77d769ff9c20d2a3b8e8296928239eawith354d742cfffcdfc886d805a08f794d3b3661e655on an Apple M4 Max, macOS 26.6.2, Rust 1.99.1 (58cf8f9c3, LLVM 23.1.1), using default release settings and thetdigestfeature.benchmarks/tdigest/support.rsfixtures for both builds. Timing usesInstantandstd::hint::black_box, with the system allocator and allocation profiling disabled. Builds have no custom Rust flags. This is a local desktop run, without CPU affinity or an isolated host.k=200and 100,000 deterministic values. CDF/PMF split points and sorted quantile ranks are evenly spaced in(0, 1). Output allocation and destruction are included.Results
Times below are whole-batch medians, not single-call latencies.
Paired change = median_i(100 * (PR_i / main_i - 1)); A/A applies the same formula to the identical baseline copy. Positive means slower. A median of paired ratios need not equal the ratio of the displayed median times. Counts were fixed after baseline calibration, so measured baseline totals can drift from the 300 ms target.The largest paired median increase in this run is 1.95% for mixed Rank, with all seven paired increases between 0.62% and 2.88%. The same workload's A/A paired range is −2.31% to +4.32%; across workloads, A/A paired medians range from −3.28% to +1.78%. The control demonstrates measurement variability, but does not prove that every candidate difference is noise or exclude a small performance effect. No speedup or universal zero-regression guarantee is claimed.
As a separate code-generation check, the CDF kernel, pairwise merge, compression loop, centroid merge, and decoder have matching normalized instruction sequences after resolving external call targets and excluding relocation addresses and alignment padding. The decoder contains 1,666 instructions in each build. This checks the named kernels, not every inlined caller or workload.
Validation
cargo x checkcargo x test: 778 passing tests, including doctests and serialization compatibility testscargo x lintStandalone fixed-work benchmark harness and runner
These temporary benchmark sources are provided here for reproducing the measurement protocol; they are not part of the library diff. Create an independent Cargo package named
tdigest-probe(version0.0.0, edition2024) with a path dependency on the checkout'sdatasketchespackage, enabling onlytdigest. The measured manifest also declares optionaldivan = "=0.1.21"andprofile = ["dep:divan"]; leave that feature disabled. Copy the checkout'sbenchmarks/tdigest/support.rsunchanged tosrc/support.rs, and use the following assrc/main.rs.Build the exact same harness against each revision with
cargo +1.99.1 build --release. Keep the executables immutable asbin/baselineandbin/candidate, then copybin/baselinetobin/control. Run the Python driver besidebin/aspython3 run.py baseline,control,candidate results. The QoS call in this driver matches the macOS measurement host; input preparation and process startup are outside the Rust timer.