Repository navigation
Conversation
…ty() A new TDigestDouble no longer allocates its centroid and buffer storage in the constructor. The buffer is allocated on the first update and grows geometrically (8, 16, ...) up to its full size; the centroid storage is allocated at the first compression or merge and grows as needed up to its full size. A heapified t-Digest is sized to its image. Compression happens at the same points as before, so results and the serialized form are unchanged. ensureCapacity() allocates the full storage up front. merge() now allocates everything before it widens min and max, so an OutOfMemoryError leaves the t-Digest unchanged. This follows the lazy allocation convention agreed on dev@. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
This applies the lazy allocation convention agreed on dev@ (vote, proposal) to
TDigestDouble. It is the Java counterpart of apache/datasketches-cpp#561. The KLL part is #800; REQ will follow.Behavior
Before this change, a new
TDigestDoubleallocated all of its storage in the constructor: centroid arrays for2k + 10centroids and a buffer four times that size (about 10 KB at k = 100).Now storage is allocated on first use and grows as values arrive, following the C++ change:
ensureCapacity(): a new public method that allocates the full storage up front. It does nothing for storage already at full size, and it doesn't change the contents.Compression happens at exactly the same points as before, so results and the serialized form don't change. The generated cross-language files are byte-identical to those from main.
The class javadoc now says that an
OutOfMemoryErrorfrom growing the storage comes fromupdate()ormerge(), not the constructor, and that the t-Digest is then unchanged.toString()shows both the full and the allocated capacities.Fix: merge order
merge()used to widen min and max before allocating its work arrays. AnOutOfMemoryErrorthere would leave a stale min or max behind, for example in an empty t-Digest. It now allocates everything first. The C++ tests found the same problem in apache/datasketches-cpp#561.Tests
The new
TDigestLazyAllocationTestchecks:ensureCapacity(), the storage never grows through 100,000 updates, and the image is byte-identical to one built without it.ensureCapacity()after updates keeps the contents, buffered values included, and later results match.ensureCapacity(), and doesn't allocate a buffer.The full suite passes (2,411 tests), and so do the t-digest cross-language checks against the local C++, Go and Rust files.
javac -Xlint:all -Xdoclint:all,-missingreports nothing in the t-digest package.🤖 Generated with Claude Code