mikemccand commented on code in PR #15938:
URL: https://github.com/apache/lucene/pull/15938#discussion_r3064659855
##########
lucene/core/src/java/org/apache/lucene/index/ReaderUtil.java:
##########
@@ -114,33 +114,25 @@ public static int[][] partitionByLeaf(ScoreDoc[] hits,
List<LeafReaderContext> l
sortedDocIds[i] = hits[i].doc;
}
Arrays.sort(sortedDocIds);
- int leafStart = 0;
+ int from = 0;
int leafIdx = 0;
- LeafReaderContext leaf = leaves.getFirst();
- int leafEnd = leaf.docBase + leaf.reader().maxDoc();
- for (int i = 0; i < sortedDocIds.length; i++) {
- int docId = sortedDocIds[i];
- while (docId >= leafEnd) {
- int count = i - leafStart;
- if (count == 0) {
- result[leafIdx] = EMPTY_INT_ARRAY;
- } else {
- result[leafIdx] = new int[count];
- System.arraycopy(sortedDocIds, leafStart, result[leafIdx], 0, count);
- }
- leafStart = i;
- leafIdx++;
- leaf = leaves.get(leafIdx);
- leafEnd = leaf.docBase + leaf.reader().maxDoc();
+ for (; leafIdx < numLeaves && from < sortedDocIds.length; leafIdx++) {
+ LeafReaderContext leaf = leaves.get(leafIdx);
+ int leafEnd = leaf.docBase + leaf.reader().maxDoc();
+ if (sortedDocIds[from] >= leafEnd) {
+ result[leafIdx] = EMPTY_INT_ARRAY;
+ continue;
+ }
+ int to = Arrays.binarySearch(sortedDocIds, from, sortedDocIds.length,
leafEnd);
+ if (to < 0) {
+ to = -to - 1;
}
+ int count = to - from;
Review Comment:
Maybe `assert count > 0` since we think we're always handling/optimizing the
empty case above?
##########
lucene/core/src/java/org/apache/lucene/index/ReaderUtil.java:
##########
@@ -114,33 +114,25 @@ public static int[][] partitionByLeaf(ScoreDoc[] hits,
List<LeafReaderContext> l
sortedDocIds[i] = hits[i].doc;
}
Arrays.sort(sortedDocIds);
- int leafStart = 0;
+ int from = 0;
int leafIdx = 0;
- LeafReaderContext leaf = leaves.getFirst();
- int leafEnd = leaf.docBase + leaf.reader().maxDoc();
- for (int i = 0; i < sortedDocIds.length; i++) {
- int docId = sortedDocIds[i];
- while (docId >= leafEnd) {
- int count = i - leafStart;
- if (count == 0) {
- result[leafIdx] = EMPTY_INT_ARRAY;
- } else {
- result[leafIdx] = new int[count];
- System.arraycopy(sortedDocIds, leafStart, result[leafIdx], 0, count);
- }
- leafStart = i;
- leafIdx++;
- leaf = leaves.get(leafIdx);
- leafEnd = leaf.docBase + leaf.reader().maxDoc();
+ for (; leafIdx < numLeaves && from < sortedDocIds.length; leafIdx++) {
+ LeafReaderContext leaf = leaves.get(leafIdx);
+ int leafEnd = leaf.docBase + leaf.reader().maxDoc();
+ if (sortedDocIds[from] >= leafEnd) {
+ result[leafIdx] = EMPTY_INT_ARRAY;
Review Comment:
I wish we had per-line code coverage pulled out to the GitHub PR (here) so
we could quickly confirm whether unit tests are covering this path... I think
we do somewhere run tests with code coverage but it's a fully separate UI maybe?
--
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]
