Skipping scorer construction for clauses with zero cost

[sgup432]

Description

Refer - #15887 for details

This skips the scorer construction for clauses with zero cost. As with the example given in above issue, if a user sends hundred of should clauses, and with most of them or ALL with zero cost, we still end up constructing the scorer leading to unnecessary CPU cycles consumption.

[sgup432]

@msfroh mind taking this a look?

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

[epotyom]

This change seem to introduce a hidden API contract change. IIUC currently, ScorerSupplier#cost has no requirement that cost must be 0 only when there are no matching docs in the segment - but with this change it effectively becomes one? That seems risky.

Ideally, when a clause matches no terms in a segment, the ScorerSupplier for it should be null. In that case BooleanWeight can already optimize - e.g. if a required clause returns null ScorerSupplier for a segment, the whole boolean query can skip that segment, which might cover your case. There was a bug in TermQuery where ScorerSupplier was non-null even when the term wasn't present in the segment; #15627 fixed this for the non-prefetch (hot index) path by making TermStates.get() return null directly instead of deferring to a lambda. I'm curious whether that covered your use case? If not, it would be worth looking at profiler snapshots to understand which lower-level operation is expensive.

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

[sgup432]

@epotyom

IIUC currently, ScorerSupplier#cost has no requirement that cost must be 0 only when there are no matching docs in the segment - but with this change it effectively becomes one? That seems risky.

Yeah I think my understanding was probably wrong here. I thought cost being 0 means no matches for sure, while cost>0 is an estimate. So the current approach taken in this PR is not right.

There was a bug in TermQuery where ScorerSupplier was non-null even when the term wasn't present in the segment; #15627 fixed this for the non-prefetch (hot index) path by making TermStates.get() return null directly instead of deferring to a lambda. I'm curious whether that covered your use case?

It might though I haven't checked it deep enough, will do. Thanks for the pointers.

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!