Merged
Conversation
Add some missing methods.
Added storage layout documentation for the compact compressed sketch.
pan3793
reviewed
Aug 19, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR implements cleanup changes for the phase2 effort, upgrading Java version to 24 and improving code quality across the DataSketches library. The changes include workflow fixes, API improvements, and comprehensive code cleanup.
Key changes:
- Upgraded all GitHub Actions workflows to use Java 24 and re-enabled push triggers
- Refactored DoublesSketch API to clearly separate compact (read-only) from updatable sketches
- Fixed binary compatibility with C++ by setting P field to zero for compact sketches
- Enhanced documentation including new compressed layout documentation and improved Javadocs
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/*.yml | Updated Java version to 24 and re-enabled push triggers |
| pom.xml | Fixed maven-javadoc-plugin configuration |
| src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java | Refactored API to separate wrap() for compact sketches and writableWrap() for updatable sketches |
| src/main/java/org/apache/datasketches/quantiles/UpdateDoublesSketch.java | Enhanced Javadoc documentation for MemorySegmentRequest handling |
| src/main/java/org/apache/datasketches/quantiles/DoublesUnionImpl.java | Added logic to handle both compact and updatable segments |
| src/main/java/org/apache/datasketches/theta/CompactOperations.java | Changed P field to 0.0 for C++ compatibility |
| src/main/java/org/apache/datasketches/theta/PreambleUtil.java | Added comprehensive documentation for compressed layout |
| test files | Updated method calls to use new API (wrap vs writableWrap) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java
Outdated
Show resolved
Hide resolved
c-dickens
approved these changes
Aug 20, 2025
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.
NOTES
The main repo is being targeted for the upcoming Java 25 LTS release, which is due out next month (Sep). Currently, however, this code is only being compiled under Java 24 in order to get work done. I do not anticipate any changes being required in the move to Java 25.
The big change from the previous release (8.0.0 using Java 21) is the migration of all code that depended on datasketches-memory to FFM (Panama). This was completed with previous PRs. However, the change to FFM has created the opportunity to refactor/remove some classes that primarily existed for DS-memory, and to carefully go through the entire code base reviewing javadocs, code comments, and improving overall code completeness and quality.
So I'm using the time between now and when Java 25 is available to do just that. If anyone one else wants to help out with this effort, you are more than welcome :)
Summary of changes
File by File change comments
ROOT DIRECTORY
.GITHUB/WORKFLOWS
auto-jdk-matrix.yml
auto-os-jdk-matrix.yml
check_cpp_files.yml
codeql-analysis.yml
javadoc.yml
pom.xml
MAIN TREE
COMMON
Util.java
HLL
package-info.java
(classic) QUANTILES
DirectCompactDoublesSketch.java
DirectUpdateDoublesSketch.java
DoublesSketch.java
DoublesUnionImpl.java
UpdateDoublesSketch.java
THETA
CompactOperations
PreambleUtil
TEST TREE
(classic) QUANTILES
DirectCompactDoublesSketchTest.java
DirectUpdateDoublesSketchTest.java
DoublesMiscTest.java
DoublesSketchTest.java
DoublesUnionImplTest.java
DoublesUtilTest.java
QuantilesSketchCrossLanguageTest.java