From aa157f18335a6f09119860cbf862ce6e218a179e Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 12 Feb 2018 14:49:49 +0100 Subject: [PATCH] LUCENE-8152: Improve consumption of doc-value iterators. --- lucene/CHANGES.txt | 3 ++ .../search/join/GlobalOrdinalsCollector.java | 10 ++----- .../search/join/GlobalOrdinalsQuery.java | 16 ++-------- .../GlobalOrdinalsWithScoreCollector.java | 20 +++---------- .../join/GlobalOrdinalsWithScoreQuery.java | 12 ++------ .../apache/lucene/search/join/JoinUtil.java | 15 ++-------- .../lucene/search/join/TermsCollector.java | 5 +--- .../search/join/TermsWithScoreCollector.java | 20 +++---------- .../lucene/search/join/TestJoinUtil.java | 10 ++----- .../handler/component/ExpandComponent.java | 11 ++----- .../solr/search/CollapsingQParserPlugin.java | 30 ++++--------------- .../solr/search/IGainTermsQParserPlugin.java | 6 +--- .../TextLogisticRegressionQParserPlugin.java | 6 +--- .../facet/FacetFieldProcessorByHashDV.java | 10 ++----- .../apache/solr/search/facet/MinMaxAgg.java | 5 +--- .../search/facet/UniqueMultiDvSlotAcc.java | 5 +--- .../solr/search/TestRankQueryPlugin.java | 6 +--- 17 files changed, 38 insertions(+), 152 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 94261fa2b02..37491584c09 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -144,6 +144,9 @@ Improvements * LUCENE-8127: Speed up rewriteNoScoring when there are no MUST clauses. (Michael Braun via Adrien Grand) +* LUCENE-8152: Improve consumption of doc-value iterators. (Horatiu Lazu via + Adrien Grand) + Bug Fixes * LUCENE-8077: Fixed bug in how CheckIndex verifies doc-value iterators. diff --git a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsCollector.java b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsCollector.java index 7b49f80bbe9..15bc4b4a5b3 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsCollector.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsCollector.java @@ -77,10 +77,7 @@ final class GlobalOrdinalsCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { long segmentOrd = docTermOrds.ordValue(); long globalOrd = segmentOrdToGlobalOrdLookup.get(segmentOrd); collectedOrds.set(globalOrd); @@ -102,10 +99,7 @@ final class GlobalOrdinalsCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { collectedOrds.set(docTermOrds.ordValue()); } } diff --git a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsQuery.java index 6aaa785bb14..8247f81352a 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsQuery.java @@ -182,11 +182,7 @@ final class GlobalOrdinalsQuery extends Query { @Override public boolean matches() throws IOException { - int docID = approximation.docID(); - if (docID > values.docID()) { - values.advance(docID); - } - if (docID == values.docID()) { + if (values.advanceExact(approximation.docID())) { final long segmentOrd = values.ordValue(); final long globalOrd = segmentOrdToGlobalOrdLookup.get(segmentOrd); if (foundOrds.get(globalOrd)) { @@ -220,14 +216,8 @@ final class GlobalOrdinalsQuery extends Query { @Override public boolean matches() throws IOException { - int docID = approximation.docID(); - if (docID > values.docID()) { - values.advance(docID); - } - if (docID == values.docID()) { - if (foundOrds.get(values.ordValue())) { - return true; - } + if (values.advanceExact(approximation.docID()) && foundOrds.get(values.ordValue())) { + return true; } return false; } diff --git a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreCollector.java b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreCollector.java index fad3f0eb6cc..fdf014b2940 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreCollector.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreCollector.java @@ -113,10 +113,7 @@ abstract class GlobalOrdinalsWithScoreCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { final int globalOrd = (int) segmentOrdToGlobalOrdLookup.get(docTermOrds.ordValue()); collectedOrds.set(globalOrd); float existingScore = scores.getScore(globalOrd); @@ -145,10 +142,7 @@ abstract class GlobalOrdinalsWithScoreCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { int segmentOrd = docTermOrds.ordValue(); collectedOrds.set(segmentOrd); float existingScore = scores.getScore(segmentOrd); @@ -258,10 +252,7 @@ abstract class GlobalOrdinalsWithScoreCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { final int globalOrd = (int) segmentOrdToGlobalOrdLookup.get(docTermOrds.ordValue()); collectedOrds.set(globalOrd); occurrences.increment(globalOrd); @@ -276,10 +267,7 @@ abstract class GlobalOrdinalsWithScoreCollector implements Collector { @Override public void collect(int doc) throws IOException { - if (doc > docTermOrds.docID()) { - docTermOrds.advance(doc); - } - if (doc == docTermOrds.docID()) { + if (docTermOrds.advanceExact(doc)) { int segmentOrd = docTermOrds.ordValue(); collectedOrds.set(segmentOrd); occurrences.increment(segmentOrd); diff --git a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreQuery.java b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreQuery.java index cf83df4ed84..cdcf070cc7c 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreQuery.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/GlobalOrdinalsWithScoreQuery.java @@ -191,11 +191,7 @@ final class GlobalOrdinalsWithScoreQuery extends Query { @Override public boolean matches() throws IOException { - int docID = approximation.docID(); - if (docID > values.docID()) { - values.advance(docID); - } - if (docID == values.docID()) { + if (values.advanceExact(approximation.docID())) { final long segmentOrd = values.ordValue(); final int globalOrd = (int) segmentOrdToGlobalOrdLookup.get(segmentOrd); if (collector.match(globalOrd)) { @@ -229,11 +225,7 @@ final class GlobalOrdinalsWithScoreQuery extends Query { @Override public boolean matches() throws IOException { - int docID = approximation.docID(); - if (docID > values.docID()) { - values.advance(docID); - } - if (docID == values.docID()) { + if (values.advanceExact(approximation.docID())) { final int segmentOrd = values.ordValue(); if (collector.match(segmentOrd)) { score = collector.score(segmentOrd); diff --git a/lucene/join/src/java/org/apache/lucene/search/join/JoinUtil.java b/lucene/join/src/java/org/apache/lucene/search/join/JoinUtil.java index d42cd2d4595..cabe7fa5112 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/JoinUtil.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/JoinUtil.java @@ -199,10 +199,7 @@ public final class JoinUtil { @Override public void collect(int doc) throws IOException { - if (doc > sortedNumericDocValues.docID()) { - sortedNumericDocValues.advance(doc); - } - if (doc == sortedNumericDocValues.docID()) { + if (sortedNumericDocValues.advanceExact(doc)) { for (int i = 0; i < sortedNumericDocValues.docValueCount(); i++) { long value = sortedNumericDocValues.nextValue(); joinValues.add(value); @@ -246,15 +243,9 @@ public final class JoinUtil { @Override public void collect(int doc) throws IOException { assert docsInOrder(doc); - int dvDocID = numericDocValues.docID(); - if (dvDocID < doc) { - dvDocID = numericDocValues.advance(doc); - } - long value; - if (dvDocID == doc) { + long value = 0; + if (numericDocValues.advanceExact(doc)) { value = numericDocValues.longValue(); - } else { - value = 0; } joinValues.add(value); if (needsScore) { diff --git a/lucene/join/src/java/org/apache/lucene/search/join/TermsCollector.java b/lucene/join/src/java/org/apache/lucene/search/join/TermsCollector.java index 0205ca35e54..9299638525e 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/TermsCollector.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/TermsCollector.java @@ -84,11 +84,8 @@ abstract class TermsCollector extends DocValuesTermsCollector { @Override public void collect(int doc) throws IOException { - if (docValues.docID() < doc) { - docValues.advance(doc); - } BytesRef term; - if (docValues.docID() == doc) { + if (docValues.advanceExact(doc)) { term = docValues.binaryValue(); } else { term = new BytesRef(BytesRef.EMPTY_BYTES); diff --git a/lucene/join/src/java/org/apache/lucene/search/join/TermsWithScoreCollector.java b/lucene/join/src/java/org/apache/lucene/search/join/TermsWithScoreCollector.java index cb2c62baa02..6cd24bcba21 100644 --- a/lucene/join/src/java/org/apache/lucene/search/join/TermsWithScoreCollector.java +++ b/lucene/join/src/java/org/apache/lucene/search/join/TermsWithScoreCollector.java @@ -96,11 +96,8 @@ abstract class TermsWithScoreCollector extends DocValuesTermsCollector @Override public void collect(int doc) throws IOException { - if (docValues.docID() < doc) { - docValues.advance(doc); - } BytesRef value; - if (docValues.docID() == doc) { + if (docValues.advanceExact(doc)) { value = docValues.binaryValue(); } else { value = new BytesRef(BytesRef.EMPTY_BYTES); @@ -155,11 +152,8 @@ abstract class TermsWithScoreCollector extends DocValuesTermsCollector @Override public void collect(int doc) throws IOException { - if (docValues.docID() < doc) { - docValues.advance(doc); - } BytesRef value; - if (docValues.docID() == doc) { + if (docValues.advanceExact(doc)) { value = docValues.binaryValue(); } else { value = new BytesRef(BytesRef.EMPTY_BYTES); @@ -207,10 +201,7 @@ abstract class TermsWithScoreCollector extends DocValuesTermsCollector @Override public void collect(int doc) throws IOException { - if (doc > docValues.docID()) { - docValues.advance(doc); - } - if (doc == docValues.docID()) { + if (docValues.advanceExact(doc)) { long ord; while ((ord = docValues.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { int termID = collectedTerms.add(docValues.lookupOrd(ord)); @@ -255,10 +246,7 @@ abstract class TermsWithScoreCollector extends DocValuesTermsCollector @Override public void collect(int doc) throws IOException { - if (doc > docValues.docID()) { - docValues.advance(doc); - } - if (doc == docValues.docID()) { + if (docValues.advanceExact(doc)) { long ord; while ((ord = docValues.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { int termID = collectedTerms.add(docValues.lookupOrd(ord)); diff --git a/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java b/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java index 088bb87b7cd..405e4814ebe 100644 --- a/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java +++ b/lucene/join/src/test/org/apache/lucene/search/join/TestJoinUtil.java @@ -1471,11 +1471,8 @@ public class TestJoinUtil extends LuceneTestCase { @Override public void collect(int doc) throws IOException { - if (doc > terms.docID()) { - terms.advance(doc); - } final BytesRef joinValue; - if (doc == terms.docID()) { + if (terms.advanceExact(doc)) { joinValue = terms.binaryValue(); } else { // missing; @@ -1540,11 +1537,8 @@ public class TestJoinUtil extends LuceneTestCase { @Override public void collect(int doc) throws IOException { - if (doc > terms.docID()) { - terms.advance(doc); - } final BytesRef joinValue; - if (doc == terms.docID()) { + if (terms.advanceExact(doc)) { joinValue = terms.binaryValue(); } else { // missing; diff --git a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java index 574ba72805b..8bd68a84917 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ExpandComponent.java @@ -591,10 +591,7 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia ord = -1; } } else { - if (globalDoc > docValues.docID()) { - docValues.advance(globalDoc); - } - if (globalDoc == docValues.docID()) { + if (docValues.advanceExact(globalDoc)) { ord = docValues.ordValue(); } else { ord = -1; @@ -664,12 +661,8 @@ public class ExpandComponent extends SearchComponent implements PluginInfoInitia @Override public void collect(int docId) throws IOException { - int valuesDocID = docValues.docID(); - if (valuesDocID < docId) { - valuesDocID = docValues.advance(docId); - } long value; - if (valuesDocID == docId) { + if (docValues.advanceExact(docId)) { value = docValues.longValue(); } else { value = 0; diff --git a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java index 974df9081b5..5b985475e92 100644 --- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java @@ -560,20 +560,14 @@ public class CollapsingQParserPlugin extends QParserPlugin { int ord = -1; if(this.ordinalMap != null) { //Handle ordinalMapping case - if (contextDoc > segmentValues.docID()) { - segmentValues.advance(contextDoc); - } - if (contextDoc == segmentValues.docID()) { + if (segmentValues.advanceExact(contextDoc)) { ord = (int)segmentOrdinalMap.get(segmentValues.ordValue()); } else { ord = -1; } } else { //Handle top Level FieldCache or Single Segment Case - if (globalDoc > segmentValues.docID()) { - segmentValues.advance(globalDoc); - } - if (globalDoc == segmentValues.docID()) { + if (segmentValues.advanceExact(globalDoc)) { ord = segmentValues.ordValue(); } else { ord = -1; @@ -786,14 +780,8 @@ public class CollapsingQParserPlugin extends QParserPlugin { @Override public void collect(int contextDoc) throws IOException { - - int collapseDocID = collapseValues.docID(); - if (collapseDocID < contextDoc) { - collapseDocID = collapseValues.advance(contextDoc); - } - int collapseValue; - if (collapseDocID == contextDoc) { + if (collapseValues.advanceExact(contextDoc)) { collapseValue = (int) collapseValues.longValue(); } else { collapseValue = 0; @@ -1022,10 +1010,7 @@ public class CollapsingQParserPlugin extends QParserPlugin { ord = (int)segmentOrdinalMap.get(segmentValues.ordValue()); } } else { - if (globalDoc > segmentValues.docID()) { - segmentValues.advance(globalDoc); - } - if (globalDoc == segmentValues.docID()) { + if (segmentValues.advanceExact(globalDoc)) { ord = segmentValues.ordValue(); } } @@ -1197,13 +1182,8 @@ public class CollapsingQParserPlugin extends QParserPlugin { } public void collect(int contextDoc) throws IOException { - int collapseDocID = collapseValues.docID(); - if (collapseDocID < contextDoc) { - collapseDocID = collapseValues.advance(contextDoc); - } - int collapseKey; - if (collapseDocID == contextDoc) { + if (collapseValues.advanceExact(contextDoc)) { collapseKey = (int) collapseValues.longValue(); } else { collapseKey = 0; diff --git a/solr/core/src/java/org/apache/solr/search/IGainTermsQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/IGainTermsQParserPlugin.java index 053f50f4055..c9e14344632 100644 --- a/solr/core/src/java/org/apache/solr/search/IGainTermsQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/IGainTermsQParserPlugin.java @@ -125,12 +125,8 @@ public class IGainTermsQParserPlugin extends QParserPlugin { public void collect(int doc) throws IOException { super.collect(doc); ++count; - int valuesDocID = leafOutcomeValue.docID(); - if (valuesDocID < doc) { - valuesDocID = leafOutcomeValue.advance(doc); - } int value; - if (valuesDocID == doc) { + if (leafOutcomeValue.advanceExact(doc)) { value = (int) leafOutcomeValue.longValue(); } else { value = 0; diff --git a/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java index 842f746c56a..5d3bb46d4f2 100644 --- a/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java @@ -150,12 +150,8 @@ public class TextLogisticRegressionQParserPlugin extends QParserPlugin { } public void collect(int doc) throws IOException{ - int valuesDocID = leafOutcomeValue.docID(); - if (valuesDocID < doc) { - valuesDocID = leafOutcomeValue.advance(doc); - } int outcome; - if (valuesDocID == doc) { + if (leafOutcomeValue.advanceExact(doc)) { outcome = (int) leafOutcomeValue.longValue(); } else { outcome = 0; diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByHashDV.java b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByHashDV.java index d85b3af2dc8..8353cdd077b 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByHashDV.java +++ b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByHashDV.java @@ -387,10 +387,7 @@ class FacetFieldProcessorByHashDV extends FacetFieldProcessor { @Override public void collect(int segDoc) throws IOException { - if (segDoc > values.docID()) { - values.advance(segDoc); - } - if (segDoc == values.docID()) { + if (values.advanceExact(segDoc)) { long l = values.nextValue(); // This document must have at least one value collectValFirstPhase(segDoc, l); for (int i = 1; i < values.docValueCount(); i++) { @@ -418,10 +415,7 @@ class FacetFieldProcessorByHashDV extends FacetFieldProcessor { @Override public void collect(int segDoc) throws IOException { - if (segDoc > values.docID()) { - values.advance(segDoc); - } - if (segDoc == values.docID()) { + if (values.advanceExact(segDoc)) { collectValFirstPhase(segDoc, values.longValue()); } } diff --git a/solr/core/src/java/org/apache/solr/search/facet/MinMaxAgg.java b/solr/core/src/java/org/apache/solr/search/facet/MinMaxAgg.java index ac8bf0bdf3e..8d4dc4dee19 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/MinMaxAgg.java +++ b/solr/core/src/java/org/apache/solr/search/facet/MinMaxAgg.java @@ -335,10 +335,7 @@ public class MinMaxAgg extends SimpleAggValueSource { @Override public void collect(int doc, int slotNum) throws IOException { - if (doc > subDv.docID()) { - subDv.advance(doc); - } - if (doc == subDv.docID()) { + if (subDv.advanceExact(doc)) { int segOrd = subDv.ordValue(); int ord = toGlobal==null ? segOrd : (int)toGlobal.get(segOrd); if ((ord - slotOrd[slotNum]) * minmax < 0 || slotOrd[slotNum]==MISSING) { diff --git a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java index 02d457fe412..af419a4d96a 100644 --- a/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java +++ b/solr/core/src/java/org/apache/solr/search/facet/UniqueMultiDvSlotAcc.java @@ -71,10 +71,7 @@ class UniqueMultiDvSlotAcc extends UniqueSlotAcc { @Override public void collect(int doc, int slotNum) throws IOException { - if (doc > subDv.docID()) { - subDv.advance(doc); - } - if (doc == subDv.docID()) { + if (subDv.advanceExact(doc)) { int segOrd = (int) subDv.nextOrd(); assert segOrd >= 0; diff --git a/solr/core/src/test/org/apache/solr/search/TestRankQueryPlugin.java b/solr/core/src/test/org/apache/solr/search/TestRankQueryPlugin.java index 19373f01dfb..fa74fb44386 100644 --- a/solr/core/src/test/org/apache/solr/search/TestRankQueryPlugin.java +++ b/solr/core/src/test/org/apache/solr/search/TestRankQueryPlugin.java @@ -693,12 +693,8 @@ public class TestRankQueryPlugin extends QParserPlugin { public void setScorer(Scorer scorer) throws IOException {} public void collect(int doc) throws IOException { - int valuesDocID = values.docID(); - if (valuesDocID < doc) { - valuesDocID = values.advance(doc); - } long value; - if (valuesDocID == doc) { + if (values.advanceExact(doc)) { value = values.longValue(); } else { value = 0;