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

Pruning of estimating the point value count since BooleanScorerSupplier #13554

Open
kkewwei opened this issue Jul 9, 2024 · 2 comments · May be fixed by #13988
Open

Pruning of estimating the point value count since BooleanScorerSupplier #13554

kkewwei opened this issue Jul 9, 2024 · 2 comments · May be fixed by #13988

Comments

@kkewwei
Copy link
Contributor

kkewwei commented Jul 9, 2024

Description

In #13199, we add isEstimatedPointCountGreaterThanOrEqualTo to dynamic pruning in the point value, there also too many functions call estimatePointCount directly, dynamic pruning is not used.

public final long estimatePointCount(IntersectVisitor visitor) {

One of my ideas is pruning since BooleanScorerSupplier:

The example is as follow:

   long leadCost = Long.MAX_VALUE;
    leadCost = subs.get(Occur.MUST).stream().mapToLong(ScorerSupplier::cost(leadCost)).min().orElse(Long.MAX_VALUE);
    leadCost =
        subs.get(Occur.FILTER).stream().mapToLong(ScorerSupplier::cost(leadCost)).min().orElse(leadCost);

If it's a good idea, if is, I'm pleasure to implement.

@kkewwei kkewwei changed the title Pruning of estimating the point value count from BooleanScorerSupplier Pruning of estimating the point value count since BooleanScorerSupplier Jul 9, 2024
@jpountz
Copy link
Contributor

jpountz commented Jul 10, 2024

The idea makes sense to me, but I worry that it wouldn't look good API-wise. I also imagine that the gains would be lower than in #13199 since Weight#scorerSupplier is called one time per segment while comparators used to estimate the point count multiple times per segment.

@kkewwei
Copy link
Contributor Author

kkewwei commented Jul 10, 2024

@jpountz, thank you for reply.

I will do benchmark if it's useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants