Jackie-Jiang commented on pull request #6262: URL: https://github.com/apache/incubator-pinot/pull/6262#issuecomment-730730910
@siddharthteotia Please check my reply under the comments. I think the tests are quite thorough, and covers all the bits and different start indices. For your questions: > Is it correct to say that new bulk read32() will be used here only if the docIDs array has sequential (consecutive) ids. Otherwise, we will use the new single read APIs? Yes > The readUnchecked() has the potential to segfault / core dump? If you use readUnchecked() for the last 2 values, it can read over the boundary (explained in the javadoc). This is handled within the `FixedBitSVForwardIndexReaderV2` > There is a chance that perf wise, we might see -ve impact due to memcpy (especially for high throughput use cases). So for this API at least, I don't think JMH is a reasonable indicator of better performance always. It is worthwhile to do validation on a prod-like workload. Why is memcpy involved here? The new reader won't read extra values because the bulk read is only used for sequential ids. I already asked @pradeepgv42 to try this, and he confirmed up to 45% query latency drop comparing with the current reader. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
