gsmiller commented on code in PR #13692:
URL: https://github.com/apache/lucene/pull/13692#discussion_r1733650732
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene912/Lucene912PostingsReader.java:
##########
@@ -212,13 +213,74 @@ static void prefixSum(long[] buffer, int count, long
base) {
}
}
- static int findFirstGreater(long[] buffer, int target, int from) {
- for (int i = from; i < BLOCK_SIZE; ++i) {
- if (buffer[i] >= target) {
- return i;
+ private static boolean assertDocBuffer(long[] values, int start) {
+ assert values.length == BLOCK_SIZE + BINARY_SEARCH_WINDOW_SIZE;
+ assert values[BLOCK_SIZE] == DocIdSetIterator.NO_MORE_DOCS;
+ assert start < BLOCK_SIZE;
+
+ int endOffset;
+ if (values[0] == DocIdSetIterator.NO_MORE_DOCS) {
+ endOffset = 0;
+ } else {
+ endOffset = -1;
+ for (int i = 1; i < values.length; ++i) {
+ assert values[i] > values[i - 1] : Arrays.toString(values);
+ if (values[i] == DocIdSetIterator.NO_MORE_DOCS) {
+ endOffset = i;
+ break;
+ }
}
}
- return BLOCK_SIZE;
+ for (int i = 0; i < BINARY_SEARCH_WINDOW_SIZE; ++i) {
+ assert values[endOffset + i] == DocIdSetIterator.NO_MORE_DOCS;
+ }
+ return true;
+ }
+
+ /**
+ * Return the first index in sorted array {@code buffer} whose value is
greater than or equal to
+ * {@code target}. For correctness, it requires the last 4 values to be set
to {@code
+ * NO_MORE_DOCS}.
+ */
+ static int findNextGEQ(long[] values, long target, int start) {
+ assert assertDocBuffer(values, start);
+
+ if (values[start] >= target) {
+ // Surprisingly this is a likely condition in practice, so optimizing
for it helps.
+ return start;
+ }
+
+ // We just ruled out that our target index is at `start`.
+ start += 1;
+
+ // Now find the first interval of 4 values that contains our target.
+ for (int i = start;
+ i + BINARY_SEARCH_WINDOW_SIZE <= values.length;
+ i += BINARY_SEARCH_WINDOW_SIZE) {
+ if (values[i + BINARY_SEARCH_WINDOW_SIZE - 1] >= target) {
+ start = i;
+ break;
+ }
+ }
Review Comment:
I probably have an off-by-one bug in here somewhere, but I wonder if we
could avoid the `- 1` on each conditional check if we changed the loop to this?
```suggestion
for (int lastChecked = start - 1;
lastChecked + BINARY_SEARCH_WINDOW_SIZE < values.length;
lastChecked += BINARY_SEARCH_WINDOW_SIZE) {
if (values[lastChecked + BINARY_SEARCH_WINDOW_SIZE] >= target) {
start = lastChecked + 1;
break;
}
}
```
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene912/Lucene912PostingsReader.java:
##########
@@ -212,13 +213,74 @@ static void prefixSum(long[] buffer, int count, long
base) {
}
}
- static int findFirstGreater(long[] buffer, int target, int from) {
- for (int i = from; i < BLOCK_SIZE; ++i) {
- if (buffer[i] >= target) {
- return i;
+ private static boolean assertDocBuffer(long[] values, int start) {
+ assert values.length == BLOCK_SIZE + BINARY_SEARCH_WINDOW_SIZE;
+ assert values[BLOCK_SIZE] == DocIdSetIterator.NO_MORE_DOCS;
+ assert start < BLOCK_SIZE;
+
+ int endOffset;
+ if (values[0] == DocIdSetIterator.NO_MORE_DOCS) {
+ endOffset = 0;
+ } else {
+ endOffset = -1;
+ for (int i = 1; i < values.length; ++i) {
+ assert values[i] > values[i - 1] : Arrays.toString(values);
+ if (values[i] == DocIdSetIterator.NO_MORE_DOCS) {
+ endOffset = i;
+ break;
+ }
}
}
- return BLOCK_SIZE;
+ for (int i = 0; i < BINARY_SEARCH_WINDOW_SIZE; ++i) {
+ assert values[endOffset + i] == DocIdSetIterator.NO_MORE_DOCS;
+ }
+ return true;
+ }
+
+ /**
+ * Return the first index in sorted array {@code buffer} whose value is
greater than or equal to
Review Comment:
minor: I think you mean `{@code values}` here not `buffer`?
--
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]
