Store numeric column min/max as primitives instead of boxed Comparables - #19479
Open
xiangfu0 wants to merge 2 commits into
Open
Store numeric column min/max as primitives instead of boxed Comparables#19479xiangfu0 wants to merge 2 commits into
xiangfu0 wants to merge 2 commits into
Conversation
This was referenced Sep 6, 2026
Open
Open
xiangfu0
added
performance
…etadataImpl A server retains one ColumnMetadataImpl per (segment, column) for the segment lifetime, so a wide external table of 1000 Parquet-backed nullable INT columns pays for it 1000 times. Two changes take the retained graph from ~104 to ~64 bytes per column, measured by allocating 500k instances with a shared FieldSpec and diffing the used heap after a forced GC: - Shared column shape. totalDocs, totalNumberOfEntries, maxNumberOfMultiValues and bitsPerElement are not column-distinguishing: build() pins the two MV ints to totalDocs and 0 for every SV column, and bitsPerElement is UNAVAILABLE for every raw column. They move into an immutable SharedShape behind one ref, interned through a weak interner keyed by its own equals/hashCode exactly like the existing FIELD_SPEC_INTERNER, so the SV columns of a segment share at most ~34 instances however wide it is (one for a raw external table) and segments with the same row count share across the JVM. - Primitive min/max. The min/max of a fixed-width stored type is stored as raw bits and boxed only inside getMinValue()/getMaxValue(), which drops ~30 bytes and two surviving objects per numeric column. FLOAT/DOUBLE go through floatToIntBits/doubleToLongBits so NaN and -0.0 keep comparing exactly as Float.equals does. STRING, BYTES, BIG_DECIMAL and COMPLEX min/max stay object refs, as does a value whose class is not the box class of its stored type, so no Builder caller can lose a value. The two longs the values need would have grown the object, so they are a union: a fixed-width column derives its three element lengths from storedType.size() and its multi-value count, and a var-width column packs those lengths into the same two words instead of its (object) min/max. Net layout, verified by measurement: 72 -> 64 bytes, and no column type regresses. No on-disk format, public signature or REST payload changes: every getter keeps its signature and value, the segment-metadata JSON is bean-serialized from the same getter set, and _flags widening from byte to short is private. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… instead Review of 8eab380 found the SharedShape indirection to be a net regression for every column whose shape tuple is not shared with another, and to run its interner on the query path. Both are fixed by dropping it; the primitive min/max union, which is where the win actually is, stays. Measured with 500k instances built from one shared FieldSpec, used heap diffed after a forced GC (SerialGC, two allocation counts so the slope cancels the constant offset), with the instance size read exactly off the field offsets: variant instance raw INT SV MV INT SV STRING (one shape) (distinct) (dict) parent of 8eab380 72 B 104 101 72 8eab380 (SharedShape) 64 B 64 140 64 no shape, four ints inline 80 B 83 83 80 this commit 72 B 72 72 72 An unshared shape costs a 32-byte object plus its ~44-byte weak interner entry, so a multi-value column paid ~36 bytes more than before 8eab380. Simply putting the four ints back inline pushes the object from 72 to 80 bytes, which instead regresses every var-width column by 8. Packing bitsPerElement into the flags word - which widens from short to int and takes the field with it - frees the int that keeps the object at 72, so no column type regresses against the parent commit, while a numeric column still sheds its two min/max boxes (104 -> 72). Against 8eab380 this costs 8 bytes per column on the wide raw-INT external table it targeted (64 -> 72); that is the price of the multi-value case not costing 76. bitsPerElement is written by the segment creator as getNumBitsPerValue(...) and never exceeds Integer.SIZE, but it is read verbatim from metadata.properties and the Builder is public, so a value outside the 23-bit encodable range falls back to the Extras holder (whose int fits in the padding its four refs leave) rather than being truncated. Builder#build() is not a segment-load-only path: IndexSegment#getDataSource( String, Schema) rebuilds a virtual column's metadata on every call, so the interner ran per query per segment. It is gone, and the constraint is now documented on Builder. A FLOAT/DOUBLE NaN min/max comes back canonicalized, because floatToIntBits collapses the payload and signalling bit. That is kept rather than switched to the raw variants, so equals() on the words keeps agreeing with Float.equals, and it is now documented on getMinValue() and pinned by a test. No on-disk format, public signature or REST payload changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xiangfu0
force-pushed
the
xiangfu0/data-3221-8-primitive-minmax
branch
from
September 9, 2026 01:56
2c01e87 to
42fc38a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## xiangfu0/data-3221-7-slim-metadata-lazy-schema #19479 +/- ##
====================================================================================
+ Coverage 67.74% 67.76% +0.02%
Complexity 1430 1430
====================================================================================
Files 3492 3492
Lines 224887 224935 +48
Branches 35514 35529 +15
====================================================================================
+ Hits 152344 152436 +92
+ Misses 60513 60468 -45
- Partials 12030 12031 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
What
ColumnMetadataImplheld min and max asComparablereferences, so a numeric column retained two boxes it never needed. They are now stored as raw bits in twolongwords and boxed only insidegetMinValue()/getMaxValue(); columns with a variable-width stored type keep object references and reuse the same two words for their three element-length values, which is what makes the change shrink the object rather than grow it.FLOAT/DOUBLEgo throughfloatToIntBits/doubleToLongBitssoNaNcompares equal and-0.0stays distinct from0.0, matchingFloat.equals/Double.equals; a value whose class does not match its stored type falls back to the reference so nothing can be mistranslated.bitsPerElementmoves into the flags word, with values outside the encodable range falling back to the existing lazily allocated holder.Measured with 500k instances sharing one field spec, used-heap delta after forced GC: a nullable INT column goes from 104 to 72 bytes, a STRING column stays at 72, and nothing regresses for multi-value or mixed-shape columns.
Worth knowing:
getMinValue()/getMaxValue()on a fixed-width column now return a freshly boxed value per call rather than the same instance. Values stayequal()and of the same class; only reference identity across calls is no longer guaranteed, and no production code depended on it.Tests
ColumnMetadataImplTest: min and max round-trip for INT, LONG, FLOAT, DOUBLE, BOOLEAN, TIMESTAMP, STRING, JSON, BYTES and BIG_DECIMAL including the min-max-invalid flag and null cases;NaNis canonicalized but stays equal;bitsPerElementround-trips across 11 values including both sides of the encodable boundary and both overflow signs, asserting the flags do not bleed into it.Why
A server keeps one metadata object graph per (segment, column) for as long as the segment is loaded, so on wide tables the per-column footprint decides how many segments a server can hold. Measured end to end on a 1000-column segment, this series takes the heap retained at load from 4.08 MB to 0.175 MB per segment (4,080 to 174 bytes per column), with a fully compacting collector on both sides. No on-disk format change, the
/tables/{table}/segments/{segment}/metadataJSON stays byte-identical, and every public and SPI signature keeps working.Stack
Part 8 of 9, based on #19478. Review only this part's own commits; the earlier parts account for the rest of the diff.