Skip to content

Commit

Permalink
LUCENE-9599 Disable sort optim on index sort
Browse files Browse the repository at this point in the history
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 apache#1351
  • Loading branch information
mayya-sharipova committed Nov 10, 2020
1 parent d111039 commit ae33ff2
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,40 +46,28 @@ public abstract class TopFieldCollector extends TopDocsCollector<Entry> {
// 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<Entry> 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<Entry> 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();
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ae33ff2

Please sign in to comment.