LUCENE-4236: cleanup/optimize BooleanScorer in-order creation

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1596640 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -185,6 +185,9 @@ Optimizations
 * LUCENE-5670: Add skip/FinalOutput to FST Outputs.  (Christian
   Ziech via Mike McCandless).
 
+* LUCENE-4236: Optimize BooleanQuery's in-order scoring. This speeds up
+  some types of boolean queries.  (Robert Muir)
+
 Bug fixes
 
 * LUCENE-5673: MMapDirectory: Work around a "bug" in the JDK that throws

lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java

@@ -19,6 +19,7 @@ package org.apache.lucene.search;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -341,6 +342,11 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
     @Override
     public Scorer scorer(AtomicReaderContext context, Bits acceptDocs)
         throws IOException {
+      // initially the user provided value,
+      // but if minNrShouldMatch == optional.size(),
+      // we will optimize and move these to required, making this 0
+      int minShouldMatch = minNrShouldMatch;
+
       List<Scorer> required = new ArrayList<>();
       List<Scorer> prohibited = new ArrayList<>();
       List<Scorer> optional = new ArrayList<>();
@@ -360,34 +366,69 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
           optional.add(subScorer);
         }
       }
-
+      
-      if (required.size() == 0 && optional.size() == 0) {
+      // scorer simplifications:
+      
+      if (optional.size() == minShouldMatch) {
+        // any optional clauses are in fact required
+        required.addAll(optional);
+        optional.clear();
+        minShouldMatch = 0;
+      }
+      
+      if (required.isEmpty() && optional.isEmpty()) {
         // no required and optional clauses.
         return null;
-      } else if (optional.size() < minNrShouldMatch) {
+      } else if (optional.size() < minShouldMatch) {
         // either >1 req scorer, or there are 0 req scorers and at least 1
         // optional scorer. Therefore if there are not enough optional scorers
         // no documents will be matched by the query
         return null;
       }
       
-      // simple conjunction
+      // three cases: conjunction, disjunction, or mix
-      if (optional.size() == 0 && prohibited.size() == 0) {
+      
-        float coord = disableCoord ? 1.0f : coord(required.size(), maxCoord);
+      // pure conjunction
-        return new ConjunctionScorer(this, required.toArray(new Scorer[required.size()]), coord);
+      if (optional.isEmpty()) {
+        return excl(req(required, disableCoord), prohibited);
       }
       
-      // simple disjunction
+      // pure disjunction
-      if (required.size() == 0 && prohibited.size() == 0 && minNrShouldMatch <= 1 && optional.size() > 1) {
+      if (required.isEmpty()) {
-        float coord[] = new float[optional.size()+1];
+        return excl(opt(optional, minShouldMatch, disableCoord), prohibited);
-        for (int i = 0; i < coord.length; i++) {
+      }
-          coord[i] = disableCoord ? 1.0f : coord(i, maxCoord);
+      
+      // conjunction-disjunction mix:
+      // we create the required and optional pieces with coord disabled, and then
+      // combine the two: if minNrShouldMatch > 0, then its a conjunction: because the
+      // optional side must match. otherwise its required + optional, factoring the
+      // number of optional terms into the coord calculation
+      
+      Scorer req = excl(req(required, true), prohibited);
+      Scorer opt = opt(optional, minShouldMatch, true);
+
+      // TODO: clean this up: its horrible
+      if (disableCoord) {
+        if (minShouldMatch > 0) {
+          return new ConjunctionScorer(this, new Scorer[] { req, opt }, 1F);
+        } else {
+          return new ReqOptSumScorer(req, opt);          
+        }
+      } else if (optional.size() == 1) {
+        if (minShouldMatch > 0) {
+          return new ConjunctionScorer(this, new Scorer[] { req, opt }, coord(required.size()+1, maxCoord));
+        } else {
+          float coordReq = coord(required.size(), maxCoord);
+          float coordBoth = coord(required.size() + 1, maxCoord);
+          return new BooleanTopLevelScorers.ReqSingleOptScorer(req, opt, coordReq, coordBoth);
+        }
+      } else {
+        if (minShouldMatch > 0) {
+          return new BooleanTopLevelScorers.CoordinatingConjunctionScorer(this, coords(), req, required.size(), opt);
+        } else {
+          return new BooleanTopLevelScorers.ReqMultiOptScorer(req, opt, required.size(), coords()); 
         }
-        return new DisjunctionSumScorer(this, optional.toArray(new Scorer[optional.size()]), coord);
       }
-      
-      // Return a BooleanScorer2
-      return new BooleanScorer2(this, disableCoord, minNrShouldMatch, required, prohibited, optional, maxCoord);
     }
     
     @Override
@@ -396,17 +437,89 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
         // BS2 (in-order) will be used by scorer()
         return false;
       }
+      int optionalCount = 0;
       for (BooleanClause c : clauses) {
         if (c.isRequired()) {
           // BS2 (in-order) will be used by scorer()
           return false;
+        } else if (!c.isProhibited()) {
+          optionalCount++;
         }
       }
       
+      if (optionalCount == minNrShouldMatch) {
+        return false; // BS2 (in-order) will be used, as this means conjunction
+      }
+      
       // scorer() will return an out-of-order scorer if requested.
       return true;
     }
     
+    private Scorer req(List<Scorer> required, boolean disableCoord) {
+      if (required.size() == 1) {
+        Scorer req = required.get(0);
+        if (!disableCoord && maxCoord > 1) {
+          return new BooleanTopLevelScorers.BoostedScorer(req, coord(1, maxCoord));
+        } else {
+          return req;
+        }
+      } else {
+        return new ConjunctionScorer(this, 
+                                     required.toArray(new Scorer[required.size()]),
+                                     disableCoord ? 1.0F : coord(required.size(), maxCoord));
+      }
+    }
+    
+    private Scorer excl(Scorer main, List<Scorer> prohibited) throws IOException {
+      if (prohibited.isEmpty()) {
+        return main;
+      } else if (prohibited.size() == 1) {
+        return new ReqExclScorer(main, prohibited.get(0));
+      } else {
+        float coords[] = new float[prohibited.size()+1];
+        Arrays.fill(coords, 1F);
+        // TODO: don't score here.
+        return new ReqExclScorer(main, 
+                                 new DisjunctionSumScorer(this, 
+                                                          prohibited.toArray(new Scorer[prohibited.size()]), 
+                                                          coords));
+      }
+    }
+    
+    private Scorer opt(List<Scorer> optional, int minShouldMatch, boolean disableCoord) throws IOException {
+      if (optional.size() == 1) {
+        Scorer opt = optional.get(0);
+        if (!disableCoord && maxCoord > 1) {
+          return new BooleanTopLevelScorers.BoostedScorer(opt, coord(1, maxCoord));
+        } else {
+          return opt;
+        }
+      } else {
+        float coords[];
+        if (disableCoord) {
+          coords = new float[optional.size()+1];
+          Arrays.fill(coords, 1F);
+        } else {
+          coords = coords();
+        }
+        if (minShouldMatch > 1) {
+          return new MinShouldMatchSumScorer(this, optional, minShouldMatch, coords);
+        } else {
+          return new DisjunctionSumScorer(this, 
+                                          optional.toArray(new Scorer[optional.size()]), 
+                                          coords);
+        }
+      }
+    }
+    
+    private float[] coords() {
+      float[] coords = new float[maxCoord+1];
+      coords[0] = 0F;
+      for (int i = 1; i < coords.length; i++) {
+        coords[i] = coord(i, maxCoord);
+      }
+      return coords;
+    }
   }
 
   @Override

lucene/core/src/java/org/apache/lucene/search/BooleanScorer2.java

@@ -1,328 +0,0 @@
-package org.apache.lucene.search;
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-
-import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.BooleanQuery.BooleanWeight;
-import org.apache.lucene.search.similarities.Similarity;
-
-/* See the description in BooleanScorer.java, comparing
- * BooleanScorer & BooleanScorer2 */
-
-/** An alternative to BooleanScorer that also allows a minimum number
- * of optional scorers that should match.
- * <br>Implements skipTo(), and has no limitations on the numbers of added scorers.
- * <br>Uses ConjunctionScorer, DisjunctionScorer, ReqOptScorer and ReqExclScorer.
- */
-class BooleanScorer2 extends Scorer {
-  
-  private final List<Scorer> requiredScorers;
-  private final List<Scorer> optionalScorers;
-  private final List<Scorer> prohibitedScorers;
-
-  private class Coordinator {
-    final float coordFactors[];
-
-    Coordinator(int maxCoord, boolean disableCoord) {
-      coordFactors = new float[optionalScorers.size() + requiredScorers.size() + 1];
-      for (int i = 0; i < coordFactors.length; i++) {
-        coordFactors[i] = disableCoord ? 1.0f : ((BooleanWeight)weight).coord(i, maxCoord);
-      }
-    }
-    
-    int nrMatchers; // to be increased by score() of match counting scorers.
-  }
-
-  private final Coordinator coordinator;
-
-  /** The scorer to which all scoring will be delegated,
-   * except for computing and using the coordination factor.
-   */
-  private final Scorer countingSumScorer;
-
-  /** The number of optionalScorers that need to match (if there are any) */
-  private final int minNrShouldMatch;
-
-  private int doc = -1;
-
-  /**
-   * Creates a {@link Scorer} with the given similarity and lists of required,
-   * prohibited and optional scorers. In no required scorers are added, at least
-   * one of the optional scorers will have to match during the search.
-   * 
-   * @param weight
-   *          The BooleanWeight to be used.
-   * @param disableCoord
-   *          If this parameter is true, coordination level matching 
-   *          ({@link Similarity#coord(int, int)}) is not used.
-   * @param minNrShouldMatch
-   *          The minimum number of optional added scorers that should match
-   *          during the search. In case no required scorers are added, at least
-   *          one of the optional scorers will have to match during the search.
-   * @param required
-   *          the list of required scorers.
-   * @param prohibited
-   *          the list of prohibited scorers.
-   * @param optional
-   *          the list of optional scorers.
-   */
-  public BooleanScorer2(BooleanWeight weight, boolean disableCoord, int minNrShouldMatch,
-      List<Scorer> required, List<Scorer> prohibited, List<Scorer> optional, int maxCoord) throws IOException {
-    super(weight);
-    if (minNrShouldMatch < 0) {
-      throw new IllegalArgumentException("Minimum number of optional scorers should not be negative");
-    }
-    this.minNrShouldMatch = minNrShouldMatch;
-
-    optionalScorers = optional;
-    requiredScorers = required;    
-    prohibitedScorers = prohibited;
-    coordinator = new Coordinator(maxCoord, disableCoord);
-    
-    countingSumScorer = makeCountingSumScorer(disableCoord);
-  }
-  
-  /** Count a scorer as a single match. */
-  private class SingleMatchScorer extends Scorer {
-    private Scorer scorer;
-    private int lastScoredDoc = -1;
-    // Save the score of lastScoredDoc, so that we don't compute it more than
-    // once in score().
-    private float lastDocScore = Float.NaN;
-
-    SingleMatchScorer(Scorer scorer) {
-      super(scorer.weight);
-      this.scorer = scorer;
-    }
-
-    @Override
-    public float score() throws IOException {
-      int doc = docID();
-      if (doc >= lastScoredDoc) {
-        if (doc > lastScoredDoc) {
-          lastDocScore = scorer.score();
-          lastScoredDoc = doc;
-        }
-        coordinator.nrMatchers++;
-      }
-      return lastDocScore;
-    }
-
-    @Override
-    public int freq() throws IOException {
-      return 1;
-    }
-
-    @Override
-    public int docID() {
-      return scorer.docID();
-    }
-
-    @Override
-    public int nextDoc() throws IOException {
-      return scorer.nextDoc();
-    }
-
-    @Override
-    public int advance(int target) throws IOException {
-      return scorer.advance(target);
-    }
-
-    @Override
-    public long cost() {
-      return scorer.cost();
-    }
-  }
-
-  private Scorer countingDisjunctionSumScorer(final List<Scorer> scorers,
-      int minNrShouldMatch) throws IOException {
-    // each scorer from the list counted as a single matcher
-    if (minNrShouldMatch > 1) {
-      return new MinShouldMatchSumScorer(weight, scorers, minNrShouldMatch) {
-        @Override 
-        public float score() throws IOException {
-          coordinator.nrMatchers += super.nrMatchers;
-          return super.score();
-        }
-      };
-    } else {
-      // we pass null for coord[] since we coordinate ourselves and override score()
-      return new DisjunctionSumScorer(weight, scorers.toArray(new Scorer[scorers.size()]), null) {
-        @Override 
-        public float score() throws IOException {
-          coordinator.nrMatchers += super.nrMatchers;
-          return (float) super.score;
-        }
-      };
-    }
-  }
-
-  private Scorer countingConjunctionSumScorer(boolean disableCoord,
-                                              List<Scorer> requiredScorers) throws IOException {
-    // each scorer from the list counted as a single matcher
-    final int requiredNrMatchers = requiredScorers.size();
-    return new ConjunctionScorer(weight, requiredScorers.toArray(new Scorer[requiredScorers.size()])) {
-      private int lastScoredDoc = -1;
-      // Save the score of lastScoredDoc, so that we don't compute it more than
-      // once in score().
-      private float lastDocScore = Float.NaN;
-      @Override public float score() throws IOException {
-        int doc = docID();
-        if (doc >= lastScoredDoc) {
-          if (doc > lastScoredDoc) {
-            lastDocScore = super.score();
-            lastScoredDoc = doc;
-          }
-          coordinator.nrMatchers += requiredNrMatchers;
-        }
-        // All scorers match, so defaultSimilarity super.score() always has 1 as
-        // the coordination factor.
-        // Therefore the sum of the scores of the requiredScorers
-        // is used as score.
-        return lastDocScore;
-      }
-    };
-  }
-
-  private Scorer dualConjunctionSumScorer(boolean disableCoord,
-                                                Scorer req1, Scorer req2) throws IOException { // non counting.
-    return new ConjunctionScorer(weight, new Scorer[] { req1, req2 });
-    // All scorers match, so defaultSimilarity always has 1 as
-    // the coordination factor.
-    // Therefore the sum of the scores of two scorers
-    // is used as score.
-  }
-
-  /** Returns the scorer to be used for match counting and score summing.
-   * Uses requiredScorers, optionalScorers and prohibitedScorers.
-   */
-  private Scorer makeCountingSumScorer(boolean disableCoord) throws IOException { // each scorer counted as a single matcher
-    return (requiredScorers.size() == 0)
-      ? makeCountingSumScorerNoReq(disableCoord)
-      : makeCountingSumScorerSomeReq(disableCoord);
-  }
-
-  private Scorer makeCountingSumScorerNoReq(boolean disableCoord) throws IOException { // No required scorers
-    // minNrShouldMatch optional scorers are required, but at least 1
-    int nrOptRequired = (minNrShouldMatch < 1) ? 1 : minNrShouldMatch;
-    Scorer requiredCountingSumScorer;
-    if (optionalScorers.size() > nrOptRequired)
-      requiredCountingSumScorer = countingDisjunctionSumScorer(optionalScorers, nrOptRequired);
-    else if (optionalScorers.size() == 1)
-      requiredCountingSumScorer = new SingleMatchScorer(optionalScorers.get(0));
-    else {
-      requiredCountingSumScorer = countingConjunctionSumScorer(disableCoord, optionalScorers);
-    }
-    return addProhibitedScorers(requiredCountingSumScorer);
-  }
-
-  private Scorer makeCountingSumScorerSomeReq(boolean disableCoord) throws IOException { // At least one required scorer.
-    if (optionalScorers.size() == minNrShouldMatch) { // all optional scorers also required.
-      ArrayList<Scorer> allReq = new ArrayList<>(requiredScorers);
-      allReq.addAll(optionalScorers);
-      return addProhibitedScorers(countingConjunctionSumScorer(disableCoord, allReq));
-    } else { // optionalScorers.size() > minNrShouldMatch, and at least one required scorer
-      Scorer requiredCountingSumScorer =
-            requiredScorers.size() == 1
-            ? new SingleMatchScorer(requiredScorers.get(0))
-            : countingConjunctionSumScorer(disableCoord, requiredScorers);
-      if (minNrShouldMatch > 0) { // use a required disjunction scorer over the optional scorers
-        return addProhibitedScorers( 
-                      dualConjunctionSumScorer( // non counting
-                              disableCoord,
-                              requiredCountingSumScorer,
-                              countingDisjunctionSumScorer(
-                                      optionalScorers,
-                                      minNrShouldMatch)));
-      } else { // minNrShouldMatch == 0
-        return new ReqOptSumScorer(
-                      addProhibitedScorers(requiredCountingSumScorer),
-                      optionalScorers.size() == 1
-                        ? new SingleMatchScorer(optionalScorers.get(0))
-                        // require 1 in combined, optional scorer.
-                        : countingDisjunctionSumScorer(optionalScorers, 1));
-      }
-    }
-  }
-  
-  /** Returns the scorer to be used for match counting and score summing.
-   * Uses the given required scorer and the prohibitedScorers.
-   * @param requiredCountingSumScorer A required scorer already built.
-   */
-  private Scorer addProhibitedScorers(Scorer requiredCountingSumScorer) throws IOException
-  {
-    return (prohibitedScorers.size() == 0)
-          ? requiredCountingSumScorer // no prohibited
-          : new ReqExclScorer(requiredCountingSumScorer,
-                              ((prohibitedScorers.size() == 1)
-                                ? prohibitedScorers.get(0)
-                                : new MinShouldMatchSumScorer(weight, prohibitedScorers)));
-  }
-
-  @Override
-  public int docID() {
-    return doc;
-  }
-  
-  @Override
-  public int nextDoc() throws IOException {
-    return doc = countingSumScorer.nextDoc();
-  }
-  
-  @Override
-  public float score() throws IOException {
-    coordinator.nrMatchers = 0;
-    float sum = countingSumScorer.score();
-    return sum * coordinator.coordFactors[coordinator.nrMatchers];
-  }
-
-  @Override
-  public int freq() throws IOException {
-    return countingSumScorer.freq();
-  }
-
-  @Override
-  public int advance(int target) throws IOException {
-    return doc = countingSumScorer.advance(target);
-  }
-  
-  @Override
-  public long cost() {
-    return countingSumScorer.cost();
-  }
-
-  @Override
-  public Collection<ChildScorer> getChildren() {
-    ArrayList<ChildScorer> children = new ArrayList<>();
-    for (Scorer s : optionalScorers) {
-      children.add(new ChildScorer(s, "SHOULD"));
-    }
-    for (Scorer s : prohibitedScorers) {
-      children.add(new ChildScorer(s, "MUST_NOT"));
-    }
-    for (Scorer s : requiredScorers) {
-      children.add(new ChildScorer(s, "MUST"));
-    }
-    return children;
-  }
-}

lucene/core/src/java/org/apache/lucene/search/BooleanTopLevelScorers.java

@@ -0,0 +1,136 @@
+package org.apache.lucene.search;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+
+/** Internal document-at-a-time scorers used to deal with stupid coord() computation */
+class BooleanTopLevelScorers {
+  
+  /** 
+   * Used when there is more than one scorer in a query, but a segment
+   * only had one non-null scorer. This just wraps that scorer directly
+   * to factor in coord().
+   */
+  static class BoostedScorer extends FilterScorer {
+    private final float boost;
+    
+    BoostedScorer(Scorer in, float boost) {
+      super(in);
+      this.boost = boost;
+    }
+
+    @Override
+    public float score() throws IOException {
+      return in.score() * boost;
+    }
+  }
+  
+  /** 
+   * Used when there are both mandatory and optional clauses, but minShouldMatch
+   * dictates that some of the optional clauses must match. The query is a conjunction,
+   * but must compute coord based on how many optional subscorers matched (freq).
+   */
+  static class CoordinatingConjunctionScorer extends ConjunctionScorer {
+    private final float coords[];
+    private final int reqCount;
+    private final Scorer req;
+    private final Scorer opt;
+    
+    CoordinatingConjunctionScorer(Weight weight, float coords[], Scorer req, int reqCount, Scorer opt) {
+      super(weight, new Scorer[] { req, opt });
+      this.coords = coords;
+      this.req = req;
+      this.reqCount = reqCount;
+      this.opt = opt;
+    }
+    
+    @Override
+    public float score() throws IOException {
+      return (req.score() + opt.score()) * coords[reqCount + opt.freq()];
+    }
+  }
+  
+  /** 
+   * Used when there are mandatory clauses with one optional clause: we compute
+   * coord based on whether the optional clause matched or not.
+   */
+  static class ReqSingleOptScorer extends ReqOptSumScorer {
+    // coord factor if just the required part matches
+    private final float coordReq;
+    // coord factor if both required and optional part matches 
+    private final float coordBoth;
+    
+    public ReqSingleOptScorer(Scorer reqScorer, Scorer optScorer, float coordReq, float coordBoth) {
+      super(reqScorer, optScorer);
+      this.coordReq = coordReq;
+      this.coordBoth = coordBoth;
+    }
+    
+    @Override
+    public float score() throws IOException {
+      int curDoc = reqScorer.docID();
+      float reqScore = reqScorer.score();
+      if (optScorer == null) {
+        return reqScore * coordReq;
+      }
+      
+      int optScorerDoc = optScorer.docID();
+      if (optScorerDoc < curDoc && (optScorerDoc = optScorer.advance(curDoc)) == NO_MORE_DOCS) {
+        optScorer = null;
+        return reqScore * coordReq;
+      }
+      
+      return optScorerDoc == curDoc ? (reqScore + optScorer.score()) * coordBoth : reqScore * coordReq;
+    }
+  }
+
+  /** 
+   * Used when there are mandatory clauses with optional clauses: we compute
+   * coord based on how many optional subscorers matched (freq).
+   */
+  static class ReqMultiOptScorer extends ReqOptSumScorer {
+    private final int requiredCount;
+    private final float coords[];
+    private final Scorer disjunction;
+    
+    public ReqMultiOptScorer(Scorer reqScorer, Scorer optScorer, int requiredCount, float coords[]) {
+      super(reqScorer, optScorer);
+      this.requiredCount = requiredCount;
+      this.coords = coords;
+      this.disjunction = optScorer;
+    }
+    
+    @Override
+    public float score() throws IOException {
+      int curDoc = reqScorer.docID();
+      float reqScore = reqScorer.score();
+      if (optScorer == null) {
+        return reqScore * coords[requiredCount];
+      }
+      
+      int optScorerDoc = optScorer.docID();
+      if (optScorerDoc < curDoc && (optScorerDoc = optScorer.advance(curDoc)) == NO_MORE_DOCS) {
+        optScorer = null;
+        return reqScore * coords[requiredCount];
+      }
+      
+      return optScorerDoc == curDoc ? (reqScore + optScorer.score()) * coords[requiredCount + disjunction.freq()] : reqScore * coords[requiredCount];
+    }
+  }
+}

lucene/core/src/java/org/apache/lucene/search/DisjunctionScorer.java

@@ -42,7 +42,7 @@ abstract class DisjunctionScorer extends Scorer {
    * Organize subScorers into a min heap with scorers generating the earliest document on top.
    */
   protected final void heapify() {
-    for (int i = (numScorers >> 1) - 1; i >= 0; i--) {
+    for (int i = (numScorers >>> 1) - 1; i >= 0; i--) {
       heapAdjust(i);
     }
   }
@@ -55,7 +55,7 @@ abstract class DisjunctionScorer extends Scorer {
     Scorer scorer = subScorers[root];
     int doc = scorer.docID();
     int i = root;
-    while (i <= (numScorers >> 1) - 1) {
+    while (i <= (numScorers >>> 1) - 1) {
       int lchild = (i << 1) + 1;
       Scorer lscorer = subScorers[lchild];
       int ldoc = lscorer.docID();

lucene/core/src/java/org/apache/lucene/search/FilterScorer.java

@@ -0,0 +1,82 @@
+package org.apache.lucene.search;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+import java.util.Collection;
+
+import org.apache.lucene.util.AttributeSource;
+
+/** 
+ * A {@code FilterScorer} contains another {@code Scorer}, which it
+ * uses as its basic source of data, possibly transforming the data along the
+ * way or providing additional functionality. The class
+ * {@code FilterScorer} itself simply implements all abstract methods
+ * of {@code Scorer} with versions that pass all requests to the
+ * contained scorer. Subclasses of {@code FilterScorer} may
+ * further override some of these methods and may also provide additional
+ * methods and fields.
+ */
+abstract class FilterScorer extends Scorer {
+  protected final Scorer in;
+  
+  public FilterScorer(Scorer in) {
+    super(in.weight);
+    this.in = in;
+  }
+  
+  @Override
+  public float score() throws IOException {
+    return in.score();
+  }
+
+  @Override
+  public int freq() throws IOException {
+    return in.freq();
+  }
+
+  @Override
+  public int docID() {
+    return in.docID();
+  }
+
+  @Override
+  public int nextDoc() throws IOException {
+    return in.nextDoc();
+  }
+
+  @Override
+  public int advance(int target) throws IOException {
+    return in.advance(target);
+  }
+
+  @Override
+  public long cost() {
+    return in.cost();
+  }
+
+  @Override
+  public Collection<ChildScorer> getChildren() {
+    return in.getChildren();
+  }
+
+  @Override
+  public AttributeSource attributes() {
+    return in.attributes();
+  }
+}

lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java

@@ -59,6 +59,8 @@ class MinShouldMatchSumScorer extends Scorer {
   /** The number of subscorers that provide the current match. */
   protected int nrMatchers = -1;
   private double score = Float.NaN;
+  
+  private final float coord[];
 
   /**
    * Construct a <code>MinShouldMatchSumScorer</code>.
@@ -72,7 +74,7 @@ class MinShouldMatchSumScorer extends Scorer {
    * <br>When minimumNrMatchers equals the number of subScorers,
    * it is more efficient to use <code>ConjunctionScorer</code>.
    */
-  public MinShouldMatchSumScorer(Weight weight, List<Scorer> subScorers, int minimumNrMatchers) throws IOException {
+  public MinShouldMatchSumScorer(Weight weight, List<Scorer> subScorers, int minimumNrMatchers, float coord[]) throws IOException {
     super(weight);
     this.nrInHeap = this.numScorers = subScorers.size();
 
@@ -105,17 +107,10 @@ class MinShouldMatchSumScorer extends Scorer {
     for (int i = 0; i < nrInHeap; i++) {
       this.subScorers[i] = this.sortedSubScorers[mm-1+i];
     }
+    this.coord = coord;
     minheapHeapify();
     assert minheapCheck();
   }
-  
-  /**
-   * Construct a <code>DisjunctionScorer</code>, using one as the minimum number
-   * of matching subscorers.
-   */
-  public MinShouldMatchSumScorer(Weight weight, List<Scorer> subScorers) throws IOException {
-    this(weight, subScorers, 1);
-  }
 
   @Override
   public final Collection<ChildScorer> getChildren() {
@@ -223,7 +218,7 @@ class MinShouldMatchSumScorer extends Scorer {
    */
   @Override
   public float score() throws IOException {
-    return (float) score;
+    return coord[nrMatchers] * (float) score;
   }
 
   @Override

lucene/core/src/java/org/apache/lucene/search/ReqExclScorer.java

@@ -111,7 +111,7 @@ class ReqExclScorer extends Scorer {
 
   @Override
   public Collection<ChildScorer> getChildren() {
-    return Collections.singleton(new ChildScorer(reqScorer, "FILTERED"));
+    return Collections.singleton(new ChildScorer(reqScorer, "MUST"));
   }
 
   @Override

lucene/core/src/java/org/apache/lucene/search/ReqOptSumScorer.java

@@ -29,8 +29,8 @@ class ReqOptSumScorer extends Scorer {
   /** The scorers passed from the constructor.
    * These are set to null as soon as their next() or skipTo() returns false.
    */
-  private Scorer reqScorer;
+  protected Scorer reqScorer;
-  private Scorer optScorer;
+  protected Scorer optScorer;
 
   /** Construct a <code>ReqOptScorer</code>.
    * @param reqScorer The required scorer. This must match.

lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java

@@ -32,56 +32,29 @@ import java.util.Collections;
  * several places, however all they have in hand is a {@link Scorer} object, and
  * might end up computing the score of a document more than once.
  */
-public class ScoreCachingWrappingScorer extends Scorer {
+public class ScoreCachingWrappingScorer extends FilterScorer {
 
-  private final Scorer scorer;
   private int curDoc = -1;
   private float curScore;
 
   /** Creates a new instance by wrapping the given scorer. */
   public ScoreCachingWrappingScorer(Scorer scorer) {
-    super(scorer.weight);
+    super(scorer);
-    this.scorer = scorer;
   }
 
   @Override
   public float score() throws IOException {
-    int doc = scorer.docID();
+    int doc = in.docID();
     if (doc != curDoc) {
-      curScore = scorer.score();
+      curScore = in.score();
       curDoc = doc;
     }
 
     return curScore;
   }
 
-  @Override
-  public int freq() throws IOException {
-    return scorer.freq();
-  }
-
-  @Override
-  public int docID() {
-    return scorer.docID();
-  }
-
-  @Override
-  public int nextDoc() throws IOException {
-    return scorer.nextDoc();
-  }
-  
-  @Override
-  public int advance(int target) throws IOException {
-    return scorer.advance(target);
-  }
-
   @Override
   public Collection<ChildScorer> getChildren() {
-    return Collections.singleton(new ChildScorer(scorer, "CACHED"));
+    return Collections.singleton(new ChildScorer(in, "CACHED"));
-  }
-
-  @Override
-  public long cost() {
-    return scorer.cost();
   }
 }

lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java

@@ -171,7 +171,8 @@ public class TestSubScorerFreqs extends LuceneTestCase {
       boolean includeOptional = occur.contains("SHOULD");
       for (int i = 0; i < maxDocs; i++) {
         Map<Query, Float> doc0 = c.docCounts.get(i);
-        assertEquals(includeOptional ? 5 : 4, doc0.size());
+        // Y doesnt exist in the index, so its not in the scorer tree
+        assertEquals(4, doc0.size());
         assertEquals(1.0F, doc0.get(aQuery), FLOAT_TOLERANCE);
         assertEquals(4.0F, doc0.get(dQuery), FLOAT_TOLERANCE);
         if (includeOptional) {
@@ -179,7 +180,8 @@ public class TestSubScorerFreqs extends LuceneTestCase {
         }
 
         Map<Query, Float> doc1 = c.docCounts.get(++i);
-        assertEquals(includeOptional ? 5 : 4, doc1.size());
+        // Y doesnt exist in the index, so its not in the scorer tree
+        assertEquals(4, doc1.size());
         assertEquals(1.0F, doc1.get(aQuery), FLOAT_TOLERANCE);
         assertEquals(1.0F, doc1.get(dQuery), FLOAT_TOLERANCE);
         if (includeOptional) {