Avoid unnecessary DisjunctionMaxBulkScorer overhead

[shimpeko]

This change inspects clause bulk scorers up front and only uses DisjunctionMaxBulkScorer if at least one clause provides a non-default BulkScorer. Otherwise, we fall back to the scorer-based path.

c88f933 made DisjunctionMaxQuery use DisjunctionMaxBulkScorer when tieBreakerMultiplier == 0 and scoreMode == TOP_SCORES. However, this bulk path primarily pays off when at least one clause implements a specialized BulkScorer.

When all clauses return DefaultBulkScorer, the bulk windowing and replay logic adds overhead while preventing effective use of minCompetitiveScore and block-max optimizations that the scorer-based DisjunctionMaxScorer supports in TOP_SCORES mode.

In such cases, falling back to the scorer-based path typically results in better performance and restores competitive-score-based skipping.

Fixes: #15658
Related PR: #14040

[shimpeko]

I realized that we cannot call ScorerSupplier.get() more than once. Need another solutions.

[shimpeko]

I realized that we cannot call ScorerSupplier.get() more than once. Need another solutions.

I updated the code not to call ScorerSupplier.get() more than once.

[uschindler]

Hi, it would be nice to not always force push, this makes reviewing hard. I have no idea if you have fixed the linter warnings. Squashing the commits is done on our side while merging, please don't do it yourself.

Otherwise the change looks fine to me. but I leave it to @jpountz as he understand the code much better.

[uschindler]

Please fix the linter warning by running "./gradlew tidy" and commit, but don't sqash!

> Task :lucene:core:checkGoogleJavaFormat FAILED
java file(s) have google-java-format violations (run './gradlew tidy' to fix). An overview diff of changes:
== /home/runner/work/lucene/lucene/lucene/core/src/java/org/apache/lucene/search/DisjunctionMaxQuery.java
@@ -181,8 +181,7 @@
 ············}
 
 ············return·new·Weight.DefaultBulkScorer(
-················new·DisjunctionMaxScorer(tieBreakerMultiplier,·scorers,·scoreMode,·Long.MAX_VALUE)
-············);
+················new·DisjunctionMaxScorer(tieBreakerMultiplier,·scorers,·scoreMode,·Long.MAX_VALUE));
 ··········}
 
 ··········@Override

[shimpeko]

Thank you for the review, and sorry about the repeated force-pushes — I was trying to clean up the commit history and should have avoided doing that during review. I won’t force-push going forward. Ran tidy and pushed 68ada56. Will wait for @jpountz's review.

[jpountz]

When all clauses return DefaultBulkScorer, the bulk windowing and replay logic adds overhead while preventing effective use of minCompetitiveScore and block-max optimizations

It's true that the windowing+replay logic isn't free but I remember that it was still better than using a Scorer which had to keep reordering a heap on every document. As far as block-max optimizations are concerned, DisjunctionBulkMaxScorer tracks the min competitive score and passes it to its sub clauses whenever scoring a window (

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

Line 81 in 7ebdb93

scorer.setMinCompetitiveScore(topLevelScorable.minCompetitiveScore);

), so this should work fine.

Let's benchmark this change with luceneutil and our dismax tasks (https://github.com/mikemccand/luceneutil/blob/main/tasks/wikinightly.tasks#L257-L276)? DisMaxTerm in particular should be a good task since TermQuery doesn't have a bulk scorer. I haven't played with this for a while, it's possible that bulk scoring isn't helping anymore.