Specialize filterCompetitiveHits when have exact 2 clauses

[HUSTERGS]

Description

This PR propose to specialize function filterCompetitiveHits when we have exact 2 scorers, in order to reduce float calculation and potential function calls

Luceneutil result on wikimediumall with searchConcurrency=0, taskCountPerCat=5, taskRepeatCount=50 after 20 iterations

                            TaskQPS baseline      StdDevQPS my_modified_version      StdDev                Pct diff p-value
                      OrHighRare      116.59      (3.1%)      116.96      (2.7%)    0.3% (  -5% -    6%) 0.734
                       OrHighMed       87.75      (2.4%)       88.83      (2.6%)    1.2% (  -3% -    6%) 0.116
                      AndHighMed       67.91      (2.3%)       69.17      (2.2%)    1.9% (  -2% -    6%) 0.009
                     AndHighHigh       27.96      (1.4%)       28.63      (2.0%)    2.4% (  -1% -    5%) 0.000
                      OrHighHigh       26.16      (1.6%)       26.97      (1.6%)    3.1% (   0% -    6%) 0.000

[github-actions]

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

[HUSTERGS]

For what it's worth, the reason of this PR is that I find filterCompetitiveHits ocuppied about 13% of flamegraph on OrHighHigh query,

Also, filterCompetitiveHits calls MathUtil.sumUpperBound in a loop, seems repeatly calculate MathUtil.sumRelativeErrorBound(numValues), (numValues is constant within the loop), I tried to optimize this, but it shows no performance difference, maybe filterCompetitiveHits is no longer the bottleneck when numValues > 2

[jpountz]

Nice speedup! I wonder if we can make it work in the general case with something like that (untested):

diff --git a/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java b/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java
index 5c677d7a464..0b1e2f30a6e 100644
--- a/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java
+++ b/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java
@@ -128,15 +128,20 @@ class ScorerUtil {
       double maxRemainingScore,
       float minCompetitiveScore,
       int numScorers) {
-    if ((float) MathUtil.sumUpperBound(maxRemainingScore, numScorers) >= minCompetitiveScore) {
+
+    // Compute minRequiredScore as the greatest float value so that (float) MathUtil.sumUpperBound(minRequiredScore + maxRemainingScore, numScorers) <= minCompetitiveScore.
+    float minRequiredScore = (float) (minCompetitiveScore - maxRemainingScore);
+    while ((float) MathUtil.sumUpperBound(minRequiredScore + maxRemainingScore, numScorers) > minCompetitiveScore) {
+      minRequiredScore = Math.nextDown(minRequiredScore);
+    }
+
+    if (minRequiredScore <= 0) {
       return;
     }
 
     int newSize = 0;
     for (int i = 0; i < buffer.size; ++i) {
-      float maxPossibleScore =
-          (float) MathUtil.sumUpperBound(buffer.scores[i] + maxRemainingScore, numScorers);
-      if (maxPossibleScore >= minCompetitiveScore) {
+      if (buffer.scores[i] >= minRequiredScore) {
         buffer.docs[newSize] = buffer.docs[i];
         buffer.scores[newSize] = buffer.scores[i];
         newSize++;

[jpountz]

FWIW I could confirm a speedup as well with this change on wiibigall.

[HUSTERGS]

Nice speedup! I wonder if we can make it work in the general case with something like that (untested):

Thanks for your reply! These change totaly make sense to me. I've also tried to merge these two code paths, but got some terrifying output when running luceneutil :( (even after I explicitly set verifyCounts to False ). I run the suggested version of code and got similar output

Maybe there is something I'm not familiar with? Or should I just change the code according to your advice? :)

[github-actions]

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

[jpountz]

This suggests that the fined~2 query doesn't drop exactly the same hits by score, which in-turn means that my suggestion is not correct and isn't actually always the smallest float that meets the sumUpperBound(minRequiredScore + maxRemainingScore, numScorers) >= minCompetitiveScore condition, we should look into fixing it. :)

[HUSTERGS]

Got it, I'll dig into this, thanks for your explaination! It helps me a lot :)