xiangfu0 commented on code in PR #19475:
URL: https://github.com/apache/pinot/pull/19475#discussion_r4106749411
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java:
##########
@@ -81,32 +104,51 @@ public
PhysicalColumnIndexContainer(SegmentDirectory.Reader segmentReader, Colum
try {
IndexReader reader =
readerProvider.createIndexReader(segmentReader, fieldIndexConfigs, metadata);
if (reader != null) {
- indexTypes.add(indexType);
- readers.add(reader);
+ readersById[indexId] = reader;
+ presentMask |= 1L << indexId;
}
} catch (IndexReaderConstraintException ex) {
LOGGER.warn("Constraint violation when indexing {} with {} index",
columnName, indexType, ex);
}
}
}
} catch (Throwable t) {
- for (IndexReader reader : readers) {
- try {
- reader.close();
- } catch (Throwable ct) {
- LOGGER.warn("Can't close reader on init error, column: " +
columnName + " reader: " + reader.getClass(), ct);
+ for (IndexReader reader : readersById) {
+ if (reader != null) {
+ try {
+ reader.close();
+ } catch (Throwable ct) {
+ LOGGER.warn("Can't close reader on init error, column: " +
columnName + " reader: " + reader.getClass(),
+ ct);
+ }
}
}
throw t;
}
- _indexTypeMap = IndexTypeMap.get(indexTypes, readers);
+ _presentMask = presentMask;
+ int numReaders = Long.bitCount(presentMask);
+ if (numReaders == 0) {
+ _readers = EMPTY_READERS;
+ } else {
+ _readers = new IndexReader[numReaders];
+ int pos = 0;
+ for (IndexReader reader : readersById) {
+ if (reader != null) {
+ _readers[pos++] = reader;
+ }
+ }
+ }
}
@Nullable
@Override
public <I extends IndexReader, T extends IndexType<?, I, ?>> I getIndex(T
indexType) {
- return _indexTypeMap.getIndex(indexType);
+ short indexId = IndexService.getInstance().getNumericId(indexType);
Review Comment:
Measured with a JMH that runs copies of the old and new `getIndex` in one
JVM, both including the real `getNumericId` call (JDK 25, Apple Silicon). With
one hot container the cost is unchanged: 24.1–24.8 ns vs 24.7–25.1 ns for a
five-lookup mix, with present lookups about 0.5 ns slower and absent ones about
1 ns faster. Over 262,144 containers visited in random order, with each
container's objects adjacent as a copying GC places them, the new layout is
faster: 58–61 ns vs 96–104 ns for the mix, and about 2x for present-only or
absent-only lookups. The cache-miss result depends on object placement: an
earlier version that put the new layout's readers between its container and
array showed it 10–25% slower. Full numbers are in the PR description; the
benchmark is not committed.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
