epotyom commented on code in PR #16268:
URL: https://github.com/apache/lucene/pull/16268#discussion_r3476643944
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/cutters/ranges/LongRangeFacetCutter.java:
##########
@@ -275,6 +363,40 @@ public boolean advanceExact(int doc) throws IOException {
return true;
}
+ /** Mirrors {@code HistogramCollector#advanceSkipper}. */
+ private void advanceSkipper(int doc) throws IOException {
+ if (doc > skipper.maxDocID(0)) {
+ skipper.advance(doc);
+ }
+ upToSameInterval = false;
+
+ if (skipper.minDocID(0) > doc) {
+ // Corner case which happens if doc doesn't have a value and is
between two intervals of the
+ // skip index. Fall back to per-doc lookups until the next block.
+ upToInclusive = skipper.minDocID(0) - 1;
+ return;
+ }
+
+ upToInclusive = skipper.maxDocID(0);
+ // Now find the highest level where all docs have a value and map to the
same interval.
+ for (int level = 0; level < skipper.numLevels(); ++level) {
+ int totalDocsAtLevel = skipper.maxDocID(level) -
skipper.minDocID(level) + 1;
+ if (skipper.docCount(level) != totalDocsAtLevel) {
+ // Some docs at this level have no value, so we can't resolve the
whole block at once.
Review Comment:
Interesting, nice catch!
> This is due to a change in the latest commit.
Does the latest commit also include the interval-tracking `rewind` change?
If so, could you please run benchmarks for the unwrapping change only?
Unwrapping adds a little bit of complexity, but if it improves performance,
I think we should keep it.
--
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]
