Skip to content

REQ: allocate the first compactor's storage on first use; add ensureCapacity() - #802

Open
leerho wants to merge 1 commit into
mainfrom
req-lazy-allocation
Open

leerho wants to merge 1 commit into
mainfrom
req-lazy-allocation

Conversation

@leerho

@leerho leerho commented Nov 12, 2026

Copy link
Copy Markdown
Member

This applies the lazy allocation convention agreed on dev@ (vote, proposal) to ReqSketch. It is the Java counterpart of apache/datasketches-cpp#575. The KLL part is #800 and the t-digest part is #801.

Behavior

  • When storage is allocated: the first compactor's storage is allocated on first use, which is the first update or the first merge of a non-empty sketch.
    • Building a sketch doesn't allocate it.
    • Heapifying an empty image doesn't allocate it.
    • reset() releases it.
    • A NaN update, or merging an empty sketch or null, is not a first use.
  • Size: the storage is the same size a new sketch used to allocate, twice the compactor's nominal capacity. Higher levels are created as before.
  • Order: the first-use allocation happens before anything else changes.
  • ReqSketch.ensureCapacity(): a new public method that allocates the storage up front. It does nothing once the storage exists.
  • Unchanged: results and the serialized form.
  • Docs: the class javadoc says that an OutOfMemoryError from the first allocation now comes from the first update or merge, not from building the sketch. ReqSketchBuilder.build() points to ensureCapacity().

Related changes from the C++ review

  • merge() now adds any levels it needs before it changes totalN, min and max. The merging of compactors themselves is unchanged.
  • ReqSketchIterator.next() now skips any level without items. Before, it assumed every level above level 0 had items. When it moved onto an empty level, it reported a non-existent item, or one too many at the end. C++ found the same problem in expansion and some more cleanup #575.
    • I couldn't produce an empty upper level through updates and merges in Java: thousands of random updates and mixed merges, both accuracy modes, never left one.
    • The new test builds such a sketch directly. It fails with the old iterator and passes with the new one.
  • Compression trigger: C++ compressed on == in update(). Java already uses >= in both update() and merge(), so there's nothing to change.

Tests

The new ReqLazyAllocationTest covers both accuracy modes. Its sketches use a silent ReqDebug, which fixes the random seed, so whole images can be compared byte for byte. It checks:

  • An empty sketch holds no storage, and none of these allocate any:
    • a NaN update;
    • merging an empty sketch or null;
    • iteration, toString and viewCompactorDetail;
    • serialization and heapifying its image.
  • ensureCapacity() allocates twice the nominal capacity, and a second call changes nothing.
  • After 100,000 updates, a sketch with ensureCapacity() and one without produce byte-identical images. Calling it after updates changes nothing.
  • The first update and the first merge allocate the storage. Merging into a new sketch gives the same image as merging into one with ensureCapacity().
  • reset() releases the storage, and the sketch works again afterwards.
  • The iterator covers exactly the retained items, with weights summing to N, for many sizes and merges. That includes a sketch whose levels 1 and 3 are empty.

The full suite passes (2,410 tests), and so do the REQ cross-language checks. javac -Xlint:all -Xdoclint:all,-missing reports nothing new; the one existing REQ warning, a misplaced @Override above a javadoc, is unchanged.

🤖 Generated with Claude Code

…apacity()

A new ReqSketch, an empty heapified one, and one after reset() no longer
allocate the storage of the first compactor. It is allocated on the
first update, or the first merge of a non-empty sketch, before anything
else changes, with the same size as before: twice the compactor's
nominal capacity. ensureCapacity() allocates it up front. Results and
the serialized form are unchanged.

merge() now grows its levels before it changes totalN, min and max.

ReqSketchIterator.next() now skips any level without items, instead of
assuming every level above level 0 has some.

This follows the lazy allocation convention agreed on dev@.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@leerho
leerho requested review from proost and tisonkun November 12, 2026 21:10
final byte lgWeight,
final boolean hra,
final int sectionSize,
final ReqDebug reqDebug,

This branch has not been deployed

No deployments
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.

2 participants