gortiz opened a new pull request, #19992: URL: https://github.com/apache/pinot/pull/19992
## Summary `FixedBitMVForwardIndexReader.getNumValuesMV()` reads the doc position from the reader `Context`, but it does not write the position back. `getDictIdMV()` and `recordDocIdByteRanges()` do write it. As a result, a scan that only counts values never takes the O(1) "next doc" path. Each call scans the chunk bitmap from the start of the chunk, so the cost for one segment is quadratic in the number of docs per chunk. This PR stores `docId` and the end offset in the `Context`, the same way `getDictIdMV()` does. ## Background A chunk holds about 2048 values. If a column has about one value per doc, a chunk holds about 2048 docs. With the context never updated, the reader scans about 1024 bitmap bits (128 bytes, one byte at a time) for each doc, instead of one `getNextSetBitOffset()` call. `COUNTMV` uses this path through `BlockValSet.getNumMVEntries()`. So does any other caller of `getNumMVEntries()` on a dictionary-encoded MV column. The quadratic loop also makes the latency unstable. Its cost depends on how C2 compiles the inlined bitmap scan, and that changes from one JVM start to the next. On the same binary, the query below took between 46 ms and 240 ms after warmup, depending on the JVM start. This happened on JDK 21 and on JDK 25. ## Results Query: `SELECT count(*), countMV(commit_author_names), countMV(label_ids) FROM github_events`, through the multi-stage engine. The data is the GitHub Events data set (546,496 rows, 4 segments) on one server with 6 CPUs. Each value is the average latency for one JVM start, after a warmup. | Build | Latency per JVM start (ms) | |---|---| | Without this fix | 232, 237, 68, 240, 235, 187, 69 (JDK 21); 140, 68, 185, 68, 235, 46, 46 (JDK 25) | | With this fix | 7.9, 7.5 (JDK 21); 7.3, 7.2 (JDK 25) | `countMV(label_ids)` alone went from 23–109 ms to about 5 ms. ## Correctness After `getNumValuesMV(docId)`, the context points at `docId`. A later call for the same doc (for example, `getDictIdMV(docId)` after a count) uses the "different chunk" path and computes the start from the chunk offset. That result is correct. ## Tests - `FixedBitMVForwardIndexTest.testNumValuesMVSharesContextWithDictIdMV` (new): it mixes `getNumValuesMV()` and `getDictIdMV()` on one context. It uses sequential, skipping, and random doc order across several chunks. - `FixedBitMVForwardIndexTest`, `InnerSegmentAggregationMultiValueQueriesTest`, `InterSegmentAggregationMultiValueQueriesTest`: pass. 🤖 Generated with [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]
