Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move bulkScorer() from Weight to ScorerSupplier #13408

Merged
merged 13 commits into from
May 27, 2024

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented May 24, 2024

This relates to #13359: we want to take advantage of the Weight#scorerSupplier call to start scheduling some I/O in the background in parallel across clauses. For this to work properly with top-level disjunctions, we need to move #bulkScorer() from Weight to ScorerSupplier as well, so that the disjunctive BooleanQuery first performs a call to Weight#scorerSupplier() on all inner clauses, and then ScorerSupplier#bulkScorer on all inner clauses.

ScorerSupplier#get and ScorerSupplier#bulkScorer only support being called once. This forced me to fix some inefficiencies in bulkScorer() implementations when we would pull scorers and then throw it away when realizing that the strategy we were planning on using was not optimal. This is why e.g. ReqExclBulkScorer now also supports prohibited clauses that produce a two-phase iterator.

@jpountz jpountz added this to the 10.0.0 milestone May 24, 2024
@jpountz
Copy link
Contributor Author

jpountz commented May 24, 2024

Note: Boolean2ScorerSupplier was renamed to BooleanScorerSupplier since it now handles both BS1 and BS2, not only BS2.

Copy link
Member

@mikemccand mikemccand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jpountz

@jpountz jpountz merged commit ddf538d into apache:main May 27, 2024
3 checks passed
@jpountz jpountz deleted the scorer_supplier_bulk branch May 27, 2024 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants