You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2019/06/28 07:51:37 UTC

[lucene-solr] branch branch_8x updated: LUCENE-8890: Improve parallel iteration of two lists of same length. (#446)

This is an automated email from the ASF dual-hosted git repository.

jpountz pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new b8ce50c  LUCENE-8890: Improve parallel iteration of two lists of same length. (#446)
b8ce50c is described below

commit b8ce50c4c1f9f073b69c82e0859bf9d57aab4655
Author: Sven Amann <in...@sven-amann.de>
AuthorDate: Fri Jun 28 09:50:37 2019 +0200

    LUCENE-8890: Improve parallel iteration of two lists of same length. (#446)
    
    The class `BooleanWeight` takes a `BooleanQuery` (a list of `BooleanClause`s) as input and maintains a list of weights corresponding to the clauses. The clauses and the weights are iterated in parallel in various places throughout the class. At these code locations, it is not obvious that these two lists always have the same length, i.e., that the parallel iteration is safe. Moreover, the parallel iteration is not well supported by the Java language, which is why this operation is imp [...]
    
    This patch joins the two lists to enable parallel iteration without managing two separate lists. This makes the code’s intent more obvious and prevents bugs due to the lists getting out of sync by a future change.
---
 .../org/apache/lucene/search/BooleanWeight.java    | 63 ++++++++++++----------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java b/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java
index af74bce..be15470 100644
--- a/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java
+++ b/lucene/core/src/java/org/apache/lucene/search/BooleanWeight.java
@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -40,8 +39,18 @@ final class BooleanWeight extends Weight {
   /** The Similarity implementation. */
   final Similarity similarity;
   final BooleanQuery query;
-  
-  final ArrayList<Weight> weights;
+
+  private static class WeightedBooleanClause {
+    final BooleanClause clause;
+    final Weight weight;
+
+    WeightedBooleanClause(BooleanClause clause, Weight weight) {
+      this.clause = clause;
+      this.weight = weight;
+    }
+  }
+
+  final ArrayList<WeightedBooleanClause> weightedClauses;
   final ScoreMode scoreMode;
 
   BooleanWeight(BooleanQuery query, IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
@@ -49,10 +58,10 @@ final class BooleanWeight extends Weight {
     this.query = query;
     this.scoreMode = scoreMode;
     this.similarity = searcher.getSimilarity();
-    weights = new ArrayList<>();
+    weightedClauses = new ArrayList<>();
     for (BooleanClause c : query) {
       Weight w = searcher.createWeight(c.getQuery(), c.isScoring() ? scoreMode : ScoreMode.COMPLETE_NO_SCORES, boost);
-      weights.add(w);
+      weightedClauses.add(new WeightedBooleanClause(c, w));
     }
   }
 
@@ -74,10 +83,9 @@ final class BooleanWeight extends Weight {
     boolean fail = false;
     int matchCount = 0;
     int shouldMatchCount = 0;
-    Iterator<BooleanClause> cIter = query.iterator();
-    for (Iterator<Weight> wIter = weights.iterator(); wIter.hasNext();) {
-      Weight w = wIter.next();
-      BooleanClause c = cIter.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause c = wc.clause;
       Explanation e = w.explain(context, doc);
       if (e.isMatch()) {
         if (c.isScoring()) {
@@ -124,11 +132,9 @@ final class BooleanWeight extends Weight {
     final int minShouldMatch = query.getMinimumNumberShouldMatch();
     List<Matches> matches = new ArrayList<>();
     int shouldMatchCount = 0;
-    Iterator<Weight> wIt = weights.iterator();
-    Iterator<BooleanClause> cIt = query.clauses().iterator();
-    while (wIt.hasNext()) {
-      Weight w = wIt.next();
-      BooleanClause bc = cIt.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause bc = wc.clause;
       Matches m = w.matches(context, doc);
       if (bc.isProhibited()) {
         if (m != null) {
@@ -188,9 +194,9 @@ final class BooleanWeight extends Weight {
   // pkg-private for forcing use of BooleanScorer in tests
   BulkScorer optionalBulkScorer(LeafReaderContext context) throws IOException {
     List<BulkScorer> optional = new ArrayList<BulkScorer>();
-    Iterator<BooleanClause> cIter = query.iterator();
-    for (Weight w  : weights) {
-      BooleanClause c =  cIter.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause c = wc.clause;
       if (c.getOccur() != Occur.SHOULD) {
         continue;
       }
@@ -221,9 +227,9 @@ final class BooleanWeight extends Weight {
   private BulkScorer requiredBulkScorer(LeafReaderContext context) throws IOException {
     BulkScorer scorer = null;
 
-    Iterator<BooleanClause> cIter = query.iterator();
-    for (Weight w  : weights) {
-      BooleanClause c =  cIter.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause c = wc.clause;
       if (c.isRequired() == false) {
         continue;
       }
@@ -293,9 +299,9 @@ final class BooleanWeight extends Weight {
     }
 
     List<Scorer> prohibited = new ArrayList<>();
-    Iterator<BooleanClause> cIter = query.iterator();
-    for (Weight w  : weights) {
-      BooleanClause c =  cIter.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause c = wc.clause;
       if (c.isProhibited()) {
         Scorer scorer = w.scorer(context);
         if (scorer != null) {
@@ -346,13 +352,14 @@ final class BooleanWeight extends Weight {
 
   @Override
   public boolean isCacheable(LeafReaderContext ctx) {
-    if (weights.size() > TermInSetQuery.BOOLEAN_REWRITE_TERM_COUNT_THRESHOLD) {
+    if (query.clauses().size() > TermInSetQuery.BOOLEAN_REWRITE_TERM_COUNT_THRESHOLD) {
       // Disallow caching large boolean queries to not encourage users
       // to build large boolean queries as a workaround to the fact that
       // we disallow caching large TermInSetQueries.
       return false;
     }
-    for (Weight w : weights) {
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
       if (w.isCacheable(ctx) == false)
         return false;
     }
@@ -368,9 +375,9 @@ final class BooleanWeight extends Weight {
       scorers.put(occur, new ArrayList<>());
     }
 
-    Iterator<BooleanClause> cIter = query.iterator();
-    for (Weight w  : weights) {
-      BooleanClause c =  cIter.next();
+    for (WeightedBooleanClause wc : weightedClauses) {
+      Weight w = wc.weight;
+      BooleanClause c = wc.clause;
       ScorerSupplier subScorer = w.scorerSupplier(context);
       if (subScorer == null) {
         if (c.isRequired()) {