Skip to content

Fix lazy decoding of frequencies in BlockImpactsDocsEnum. #12668

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.

Sign up for GitHub

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

Jump to bottom
Merged
jpountz merged 2 commits into from
Oct 13, 2023
Merged

Fix lazy decoding of frequencies in BlockImpactsDocsEnum. #12668

jpountz merged 2 commits into from
Oct 13, 2023

Conversation

@jpountz
Copy link
Contributor

@jpountz jpountz commented Oct 13, 2023

The code was written as if frequencies should be lazily decoding, except that when refilling buffers freqs were getting eagerly decoded instead of lazily.

@jpountz
Fix lazy decoding of frequencies in BlockImpactsDocsEnum.
bd09acc 
The code was written as if frequencies should be lazily decoding, except that
when refilling buffers freqs were getting eagerly decoded instead of lazily.
@jpountz
Copy link
Contributor Author

jpountz commented Oct 13, 2023

Results on wikibigall:

                            TaskQPS baseline      StdDevQPS my_modified_version      StdDev                Pct diff p-value
                 CountOrHighHigh       58.65     (15.7%)       57.03     (14.9%)   -2.8% ( -28% -   33%) 0.570
                  CountOrHighMed       91.10     (15.7%)       88.61     (14.9%)   -2.7% ( -28% -   33%) 0.573
                     CountPhrase        3.59     (12.4%)        3.52     (10.0%)   -1.9% ( -21% -   23%) 0.591
                         Prefix3      223.37      (3.8%)      220.52      (2.8%)   -1.3% (  -7% -    5%) 0.221
                       MedPhrase       23.18      (4.8%)       22.89      (2.8%)   -1.3% (  -8% -    6%) 0.312
                      HighPhrase       74.30      (5.2%)       73.39      (3.8%)   -1.2% (  -9% -    8%) 0.395
                       LowPhrase       49.93      (4.0%)       49.44      (2.9%)   -1.0% (  -7% -    6%) 0.372
               HighTermMonthSort     5321.09      (3.8%)     5284.52      (4.0%)   -0.7% (  -8% -    7%) 0.576
                CountAndHighHigh       41.05      (4.4%)       40.88      (3.3%)   -0.4% (  -7% -    7%) 0.741
                 CountAndHighMed      123.48      (3.9%)      123.17      (3.2%)   -0.3% (  -7% -    7%) 0.822
           HighTermDayOfYearSort      209.14      (2.3%)      208.73      (1.3%)   -0.2% (  -3% -    3%) 0.741
                         Respell       97.13      (1.6%)       97.09      (1.7%)   -0.0% (  -3% -    3%) 0.950
                        Wildcard       79.82      (3.8%)       79.90      (3.3%)    0.1% (  -6% -    7%) 0.927
                       CountTerm    17367.55      (6.4%)    17399.19      (5.4%)    0.2% ( -10% -   12%) 0.922
                          Fuzzy2       92.55      (1.5%)       92.80      (1.6%)    0.3% (  -2% -    3%) 0.573
                          Fuzzy1      135.19      (1.4%)      135.61      (1.7%)    0.3% (  -2% -    3%) 0.532
                        PKLookup      226.80      (2.1%)      228.05      (1.8%)    0.6% (  -3% -    4%) 0.372
                      OrHighHigh       63.37      (6.2%)       64.29      (7.7%)    1.5% ( -11% -   16%) 0.511
                          IntNRQ      137.84     (20.5%)      140.16     (20.6%)    1.7% ( -32% -   53%) 0.795
                     AndHighHigh       55.84      (3.4%)       57.03      (4.9%)    2.1% (  -6% -   10%) 0.113
                         LowTerm      846.47      (5.4%)      865.92      (7.2%)    2.3% (  -9% -   15%) 0.252
                        HighTerm      338.68      (5.3%)      347.00      (9.0%)    2.5% ( -11% -   17%) 0.294
                         MedTerm      443.60      (4.9%)      454.62      (8.3%)    2.5% ( -10% -   16%) 0.252
                      AndHighMed      125.73      (2.9%)      129.22      (4.0%)    2.8% (  -3% -    9%) 0.012
                       OrHighLow      553.99      (3.7%)      576.92      (3.9%)    4.1% (  -3% -   12%) 0.001
                       OrHighMed      165.18      (4.2%)      172.30      (6.0%)    4.3% (  -5% -   15%) 0.009
                      AndHighLow      801.37      (3.0%)      848.58      (3.7%)    5.9% (   0% -   12%) 0.000

AndHighMed, OrHighLow, OrHighMed, AndHighLow all have a noticeable speedup and a very low p-value, so the speedup is real. Queries that show a small slowdown have a high p-value, suggesting that the slowdown is noise.

@jpountz
Copy link
Contributor Author

jpountz commented Oct 13, 2023

For reference this is extracted from #12664.

romseygeek
romseygeek approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM!

@jpountz jpountz merged commit afc990c into apache:main Oct 13, 2023
@jpountz jpountz deleted the fix_lazy_freqs_decoding branch October 13, 2023 08:42
@jpountz jpountz added this to the 9.9.0 milestone Oct 13, 2023
jpountz added a commit that referenced this pull request Oct 13, 2023
@jpountz
Fix lazy decoding of frequencies in BlockImpactsDocsEnum. (#12668)
709c2f6 
The code was written as if frequencies should be lazily decoding, except that
when refilling buffers freqs were getting eagerly decoded instead of lazily.
@jpountz jpountz mentioned this pull request Oct 13, 2023
Merged
@jpountz
Copy link
Contributor Author

jpountz commented Oct 17, 2023

Even though the speedup is less pronounced than in the above luceneutil run, there seems to be an actual speedup in nightly benchmarks for boolean queries. E.g. the last 3 data points of AndHighMed are the highest throughput numbers ever recorded (though they're not much faster than on the previous days). I pushed an annotation that should show up tomorrow.

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

Reviewers

@romseygeek romseygeek romseygeek approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

9.9.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jpountz @romseygeek