From 91ec76c8f6f337c4616021a68b858f912447c871 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 11:00:20 +0100 Subject: [PATCH 1/5] LUCENE-8463: Early-terminate queries sorted by SortField.DOC --- .../lucene/search/TopFieldCollector.java | 14 ++++++++ ...TestTopFieldCollectorEarlyTermination.java | 32 +++++++++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) 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 ff2307c09f50..c5897eb8498b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -68,6 +68,20 @@ public void setScorer(Scorable scorer) throws IOException { } static boolean canEarlyTerminate(Sort searchSort, Sort indexSort) { + return canEarlyTerminateOnDocId(searchSort, indexSort) || + canEarlyTerminateOnPrefix(searchSort, indexSort); + } + + private static boolean canEarlyTerminateOnDocId(Sort searchSort, Sort indexSort) { + final SortField[] fields1 = searchSort.getSort(); + final SortField[] fields2 = indexSort.getSort(); + return fields1.length == 1 && + fields2.length == 1 && + SortField.FIELD_DOC.equals(fields1[0]) && + SortField.FIELD_DOC.equals(fields2[0]); + } + + private static boolean canEarlyTerminateOnPrefix(Sort searchSort, Sort indexSort) { final SortField[] fields1 = searchSort.getSort(); final SortField[] fields2 = indexSort.getSort(); // early termination is possible if fields1 is a prefix of fields2 diff --git a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java index b6d33dad3900..fc20601f41c1 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java @@ -102,7 +102,7 @@ else if (random().nextBoolean()) { reader = iw.getReader(); } } - + private void closeIndex() throws IOException { reader.close(); iw.close(); @@ -166,8 +166,34 @@ private void doTestEarlyTermination(boolean paging) throws IOException { closeIndex(); } } - - public void testCanEarlyTerminate() { + + public void testCanEarlyTerminateOnDocId() { + assertTrue(TopFieldCollector.canEarlyTerminate( + new Sort(SortField.FIELD_DOC), + new Sort(SortField.FIELD_DOC))); + + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(new SortField("a", SortField.Type.LONG)), + new Sort(new SortField("b", SortField.Type.LONG)))); + + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(SortField.FIELD_DOC), + new Sort(new SortField("b", SortField.Type.LONG)))); + + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(SortField.FIELD_DOC), + new Sort(new SortField("b", SortField.Type.LONG), SortField.FIELD_DOC))); + + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(new SortField("a", SortField.Type.LONG)), + new Sort(SortField.FIELD_DOC))); + + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(new SortField("a", SortField.Type.LONG), SortField.FIELD_DOC), + new Sort(SortField.FIELD_DOC))); + } + + public void testCanEarlyTerminateOnPrefix() { assertTrue(TopFieldCollector.canEarlyTerminate( new Sort(new SortField("a", SortField.Type.LONG)), new Sort(new SortField("a", SortField.Type.LONG)))); From ee4255438f41f2ed1f7652b3e0fead3be6f187d1 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 12:10:26 +0100 Subject: [PATCH 2/5] LUCENE-8463: Reduce diff with master --- .../lucene/search/TestTopFieldCollectorEarlyTermination.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java index fc20601f41c1..d112dd05f325 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java @@ -102,7 +102,7 @@ else if (random().nextBoolean()) { reader = iw.getReader(); } } - + private void closeIndex() throws IOException { reader.close(); iw.close(); @@ -166,7 +166,7 @@ private void doTestEarlyTermination(boolean paging) throws IOException { closeIndex(); } } - + public void testCanEarlyTerminateOnDocId() { assertTrue(TopFieldCollector.canEarlyTerminate( new Sort(SortField.FIELD_DOC), From 7ef9b96746503ca0672d9e65b41555bc4ae567b3 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 14:59:19 +0100 Subject: [PATCH 3/5] LUCENE-8463: Remove unnecessary sort conditions --- .../src/java/org/apache/lucene/search/TopFieldCollector.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 c5897eb8498b..49ff92c18e53 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -75,9 +75,7 @@ static boolean canEarlyTerminate(Sort searchSort, Sort indexSort) { private static boolean canEarlyTerminateOnDocId(Sort searchSort, Sort indexSort) { final SortField[] fields1 = searchSort.getSort(); final SortField[] fields2 = indexSort.getSort(); - return fields1.length == 1 && - fields2.length == 1 && - SortField.FIELD_DOC.equals(fields1[0]) && + return SortField.FIELD_DOC.equals(fields1[0]) && SortField.FIELD_DOC.equals(fields2[0]); } From c3cc2885591900d170f31d30ecc2a00fc44340e1 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Tue, 13 Nov 2018 09:45:43 +0100 Subject: [PATCH 4/5] LUCENE-8463: Don't check index sort on doc id early termination --- .../lucene/search/TopFieldCollector.java | 30 ++++++++++--------- ...TestTopFieldCollectorEarlyTermination.java | 19 +++++++++--- 2 files changed, 31 insertions(+), 18 deletions(-) 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 49ff92c18e53..325a8578015b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -68,25 +68,27 @@ public void setScorer(Scorable scorer) throws IOException { } static boolean canEarlyTerminate(Sort searchSort, Sort indexSort) { - return canEarlyTerminateOnDocId(searchSort, indexSort) || + return canEarlyTerminateOnDocId(searchSort) || canEarlyTerminateOnPrefix(searchSort, indexSort); } - private static boolean canEarlyTerminateOnDocId(Sort searchSort, Sort indexSort) { + private static boolean canEarlyTerminateOnDocId(Sort searchSort) { final SortField[] fields1 = searchSort.getSort(); - final SortField[] fields2 = indexSort.getSort(); - return SortField.FIELD_DOC.equals(fields1[0]) && - SortField.FIELD_DOC.equals(fields2[0]); + return SortField.FIELD_DOC.equals(fields1[0]); } private static boolean canEarlyTerminateOnPrefix(Sort searchSort, Sort indexSort) { - final SortField[] fields1 = searchSort.getSort(); - final SortField[] fields2 = indexSort.getSort(); - // early termination is possible if fields1 is a prefix of fields2 - if (fields1.length > fields2.length) { + if (indexSort != null) { + final SortField[] fields1 = searchSort.getSort(); + final SortField[] fields2 = indexSort.getSort(); + // early termination is possible if fields1 is a prefix of fields2 + if (fields1.length > fields2.length) { + return false; + } + return Arrays.asList(fields1).equals(Arrays.asList(fields2).subList(0, fields1.length)); + } else { return false; } - return Arrays.asList(fields1).equals(Arrays.asList(fields2).subList(0, fields1.length)); } /* @@ -111,8 +113,7 @@ public LeafCollector getLeafCollector(LeafReaderContext context) throws IOExcept final LeafFieldComparator[] comparators = queue.getComparators(context); final int[] reverseMul = queue.getReverseMul(); final Sort indexSort = context.reader().getMetaData().getSort(); - final boolean canEarlyTerminate = indexSort != null && - canEarlyTerminate(sort, indexSort); + final boolean canEarlyTerminate = canEarlyTerminate(sort, indexSort); return new MultiComparatorLeafCollector(comparators, reverseMul) { @@ -202,10 +203,11 @@ public PagingFieldCollector(Sort sort, FieldValueHitQueue queue, FieldDoc @Override public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException { docBase = context.docBase; + final int afterDoc = after.doc - docBase; final Sort indexSort = context.reader().getMetaData().getSort(); - final boolean canEarlyTerminate = indexSort != null && - canEarlyTerminate(sort, indexSort); + final boolean canEarlyTerminate = canEarlyTerminate(sort, indexSort); + return new MultiComparatorLeafCollector(queue.getComparators(context), queue.getReverseMul()) { boolean collectedAllCompetitiveHits = false; diff --git a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java index d112dd05f325..a92a100e058f 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollectorEarlyTermination.java @@ -23,6 +23,7 @@ import java.util.Random; import java.util.Set; +import com.carrotsearch.randomizedtesting.generators.RandomPicks; import org.apache.lucene.analysis.MockAnalyzer; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field.Store; @@ -39,8 +40,6 @@ import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.TestUtil; -import com.carrotsearch.randomizedtesting.generators.RandomPicks; - public class TestTopFieldCollectorEarlyTermination extends LuceneTestCase { private int numDocs; @@ -171,16 +170,24 @@ public void testCanEarlyTerminateOnDocId() { assertTrue(TopFieldCollector.canEarlyTerminate( new Sort(SortField.FIELD_DOC), new Sort(SortField.FIELD_DOC))); + + assertTrue(TopFieldCollector.canEarlyTerminate( + new Sort(SortField.FIELD_DOC), + null)); assertFalse(TopFieldCollector.canEarlyTerminate( new Sort(new SortField("a", SortField.Type.LONG)), - new Sort(new SortField("b", SortField.Type.LONG)))); + null)); assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(new SortField("a", SortField.Type.LONG)), + new Sort(new SortField("b", SortField.Type.LONG)))); + + assertTrue(TopFieldCollector.canEarlyTerminate( new Sort(SortField.FIELD_DOC), new Sort(new SortField("b", SortField.Type.LONG)))); - assertFalse(TopFieldCollector.canEarlyTerminate( + assertTrue(TopFieldCollector.canEarlyTerminate( new Sort(SortField.FIELD_DOC), new Sort(new SortField("b", SortField.Type.LONG), SortField.FIELD_DOC))); @@ -206,6 +213,10 @@ public void testCanEarlyTerminateOnPrefix() { new Sort(new SortField("a", SortField.Type.LONG)), new Sort(new SortField("a", SortField.Type.LONG), new SortField("b", SortField.Type.STRING)))); + assertFalse(TopFieldCollector.canEarlyTerminate( + new Sort(new SortField("a", SortField.Type.LONG, true)), + null)); + assertFalse(TopFieldCollector.canEarlyTerminate( new Sort(new SortField("a", SortField.Type.LONG, true)), new Sort(new SortField("a", SortField.Type.LONG, false)))); From e350f5ef61ce5e22679099efca2b24a9edd4c261 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Tue, 13 Nov 2018 09:48:42 +0100 Subject: [PATCH 5/5] LUCENE-8463: Reduce diff with master --- .../src/java/org/apache/lucene/search/TopFieldCollector.java | 2 -- 1 file changed, 2 deletions(-) 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 325a8578015b..f3a2a3bb2f37 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -203,11 +203,9 @@ public PagingFieldCollector(Sort sort, FieldValueHitQueue queue, FieldDoc @Override public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException { docBase = context.docBase; - final int afterDoc = after.doc - docBase; final Sort indexSort = context.reader().getMetaData().getSort(); final boolean canEarlyTerminate = canEarlyTerminate(sort, indexSort); - return new MultiComparatorLeafCollector(queue.getComparators(context), queue.getReverseMul()) { boolean collectedAllCompetitiveHits = false;