Copilot commented on code in PR #19480:
URL: https://github.com/apache/pinot/pull/19480#discussion_r3943279773
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java:
##########
@@ -268,17 +277,21 @@ public void addIndexSize(short indexType, long size) {
"Index size should be a non-negative integer value between 0 and " +
SIZE_MASK);
}
long typeAndSize = ((long) indexType) << 48 | (size & SIZE_MASK);
- _indexTypeSizeList.add(typeAndSize);
+ // A column has a handful of indexes, so grow by one rather than pre-size.
+ int numIndexes = _indexTypeSizes == null ? 0 : _indexTypeSizes.length;
+ long[] grown = _indexTypeSizes == null ? new long[1] :
Arrays.copyOf(_indexTypeSizes, numIndexes + 1);
+ grown[numIndexes] = typeAndSize;
Review Comment:
`addIndexSize` currently reallocates and copies the backing array on every
call (`Arrays.copyOf(..., numIndexes + 1)`), which is O(k^2) allocations/copies
per column when loading `index_map` (k = number of index entries). Since this
runs once per (segment, column) at load time, it can create a lot of
short-lived arrays for wide segments.
Consider switching to an amortized-growth approach (e.g., keep a separate
count and grow capacity geometrically) or a lazily-initialized append-friendly
structure, then trim/pack once after population if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
