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

Remove unnecessary check in critical path of MatchHashesAndScoreQuery #607

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

alexklibisz
Copy link
Owner

@alexklibisz alexklibisz commented Nov 29, 2023

Related Issue

#611

Changes

Removing an unnecessary counter check from the critical path of MatchHashesAndScoreQuery. The check originated in #170, at which point it was actually useful. Then #231 removed the original reason/necessity for this check, but it was kept around for some reason.

The check is checking whether the number of hits is less than the number of possible hits going to return true in almost all cases. The only way it returns false is if we match/hit every document in the segment. If that happens, the LSH model parameters are almost certainly suboptimal.

Though this was in the critical path, the benchmark results are almost identical, so it's more of a simplification than a perf improvement:

image

Testing and Validation

Standard CI and benchmarks

@alexklibisz alexklibisz changed the title Performance: remove unnecessary check in critical path of MatchHashesAndScoreQuery Remove unnecessary check in critical path of MatchHashesAndScoreQuery Nov 29, 2023
@alexklibisz alexklibisz marked this pull request as ready for review November 29, 2023 04:54
@alexklibisz alexklibisz merged commit 4208fa6 into main Nov 29, 2023
5 checks passed
@alexklibisz alexklibisz deleted the rm-counterLimit branch November 29, 2023 04:54
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.

1 participant