From ae33ff237f656cbed3a21f50621d9b58f862b521 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 10 Nov 2020 16:47:31 -0500 Subject: [PATCH] LUCENE-9599 Disable sort optim on index sort Disable sort optimization in comparators on index sort. Currently, if search sort is equal or a part of the index sort, we have an early termination in TopFieldCollector. But comparators are not aware of the index sort, and may run sort optimization even if the search sort is congruent with the index sort. This patch: - make leaf comparators aware that search sort is congruent with the index sort. - disables sort optimization in comparators in this case. - removes a private MultiComparatorLeafCollector class as the only class that extended that class was TopFieldLeafCollector that now incorporates the logic of the deleted class. Relates to #1351 --- .../lucene/search/FieldValueHitQueue.java | 5 ++- .../lucene/search/LeafFieldComparator.java | 6 +++ .../lucene/search/TopFieldCollector.java | 37 ++++++------------- .../search/comparators/NumericComparator.java | 8 +++- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java b/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java index a790636a45d3..08ffbb42acf1 100644 --- a/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java +++ b/lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java @@ -178,10 +178,13 @@ public int[] getReverseMul() { return reverseMul; } - public LeafFieldComparator[] getComparators(LeafReaderContext context) throws IOException { + public LeafFieldComparator[] getComparators(LeafReaderContext context, boolean indexSort) throws IOException { LeafFieldComparator[] comparators = new LeafFieldComparator[this.comparators.length]; for (int i = 0; i < comparators.length; ++i) { comparators[i] = this.comparators[i].getLeafComparator(context); + if (indexSort) { + comparators[i].usesIndexSort(); + } } return comparators; } diff --git a/lucene/core/src/java/org/apache/lucene/search/LeafFieldComparator.java b/lucene/core/src/java/org/apache/lucene/search/LeafFieldComparator.java index 97f745c2f5e5..ccf1e6094377 100644 --- a/lucene/core/src/java/org/apache/lucene/search/LeafFieldComparator.java +++ b/lucene/core/src/java/org/apache/lucene/search/LeafFieldComparator.java @@ -132,4 +132,10 @@ default DocIdSetIterator competitiveIterator() throws IOException { default void setHitsThresholdReached() throws IOException{ } + /** + * Informs this leaf comparator that the search sort is congruent with the index sort. + */ + default void usesIndexSort() { + } + } diff --git a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java index e529892e6e76..910aeb1e69c3 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -46,40 +46,28 @@ public abstract class TopFieldCollector extends TopDocsCollector { // always compare lower than a real hit; this would // save having to check queueFull on each insert - private static abstract class MultiComparatorLeafCollector implements LeafCollector { + private abstract class TopFieldLeafCollector implements LeafCollector { final LeafFieldComparator comparator; final int reverseMul; + final boolean canEarlyTerminate; Scorable scorer; + boolean collectedAllCompetitiveHits = false; - MultiComparatorLeafCollector(LeafFieldComparator[] comparators, int[] reverseMul) { + TopFieldLeafCollector(FieldValueHitQueue queue, Sort sort, LeafReaderContext context) throws IOException { + final Sort indexSort = context.reader().getMetaData().getSort(); + this.canEarlyTerminate = canEarlyTerminate(sort, indexSort); + LeafFieldComparator[] comparators = queue.getComparators(context, canEarlyTerminate); + int[] reverseMuls = queue.getReverseMul(); if (comparators.length == 1) { - this.reverseMul = reverseMul[0]; + this.reverseMul = reverseMuls[0]; this.comparator = comparators[0]; } else { this.reverseMul = 1; - this.comparator = new MultiLeafFieldComparator(comparators, reverseMul); + this.comparator = new MultiLeafFieldComparator(comparators, reverseMuls); } } - @Override - public void setScorer(Scorable scorer) throws IOException { - comparator.setScorer(scorer); - this.scorer = scorer; - } - } - - private abstract class TopFieldLeafCollector extends MultiComparatorLeafCollector { - - final boolean canEarlyTerminate; - boolean collectedAllCompetitiveHits = false; - - TopFieldLeafCollector(FieldValueHitQueue queue, Sort sort, LeafReaderContext context) throws IOException { - super(queue.getComparators(context), queue.getReverseMul()); - final Sort indexSort = context.reader().getMetaData().getSort(); - canEarlyTerminate = canEarlyTerminate(sort, indexSort); - } - void countHit(int doc) throws IOException { ++totalHits; hitsThresholdChecker.incrementHitCount(); @@ -139,7 +127,8 @@ void collectAnyHit(int doc, int hitsCollected) throws IOException { @Override public void setScorer(Scorable scorer) throws IOException { - super.setScorer(scorer); + this.scorer = scorer; + comparator.setScorer(scorer); minCompetitiveScore = 0f; updateMinCompetitiveScore(scorer); if (minScoreAcc != null) { @@ -154,8 +143,6 @@ public DocIdSetIterator competitiveIterator() throws IOException { } - // TODO: remove this code when all bulk scores similar to {@code DefaultBulkScorer} use collectors' iterator, - // as early termination should be implemented in their respective comparators and removed from a collector static boolean canEarlyTerminate(Sort searchSort, Sort indexSort) { return canEarlyTerminateOnDocId(searchSort) || canEarlyTerminateOnPrefix(searchSort, indexSort); diff --git a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java index fa5f2679cd3a..f3eac8ddf6a0 100644 --- a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java +++ b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java @@ -71,11 +71,11 @@ public void setSingleSort() { public abstract class NumericLeafComparator implements LeafFieldComparator { protected final NumericDocValues docValues; private final PointValues pointValues; - private final boolean enableSkipping; // if skipping functionality should be enabled private final int maxDoc; private final byte[] minValueAsBytes; private final byte[] maxValueAsBytes; + private boolean enableSkipping; // if skipping functionality should be enabled private DocIdSetIterator competitiveIterator; private long iteratorCost; private int maxDocVisited = 0; @@ -104,6 +104,12 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String return DocValues.getNumeric(context.reader(), field); } + @Override + public void usesIndexSort() { + // disable skipping functionality on index sort, as early termination in this case is handled in TopFieldCollector + this.enableSkipping = false; + } + @Override public void setBottom(int slot) throws IOException { queueFull = true; // if we are setting bottom, it means that we have collected enough hits