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

Lucene query improvements #311

Merged
merged 7 commits into from
Jun 13, 2022
Merged

Conversation

bryanlb
Copy link
Contributor

@bryanlb bryanlb commented Jun 2, 2022

@bryanlb bryanlb requested review from mansu and vthacker June 2, 2022 22:28
@bryanlb

This comment was marked as outdated.

@bryanlb bryanlb force-pushed the bburkholder/parallel-segment-query branch from 16644d9 to e82bddd Compare June 2, 2022 22:46
@mansu
Copy link
Contributor

mansu commented Jun 3, 2022

A few questions about the benchmarks:

  • Should the benchmarks be updated with the index sorting results?

  • From the benchmarks calculating histogram takes roughly 2x the time than getting the topK results.
    0 results, 60 buckets
    Iteration 1: 38.235 ops/s
    500 results, 0 buckets
    Iteration 1: 17.951 ops/s

But calculating both histogram and topK takes less time than either:
500 results, 60 buckets
Iteration 1: 14.788 ops/s

I am curious how we can explain the speed up? Could the speed up be due to a bug in our code? Can you ensure that we have unit tests(existing or new) that test all of these outputs to make sure the outputs match the expected result?

@bryanlb bryanlb force-pushed the bburkholder/parallel-segment-query branch from 6d94ce2 to 72c32e8 Compare June 3, 2022 18:31
@bryanlb
Copy link
Contributor Author

bryanlb commented Jun 3, 2022

Updated benchmarks:

New Code:
  * 500,60 - 15.263 ops/s
  * 500,0 - 68.357 ops/s
  * 0,60 - 32.471 ops/s

Master:
  * 500,60 - 14.137 op/s
  * 500,0 - 18.546 op/s
  * 0,60 - 39.498 op/s

vthacker
vthacker previously approved these changes Jun 3, 2022
@bryanlb bryanlb force-pushed the bburkholder/parallel-segment-query branch from 21ca60b to 9705487 Compare June 13, 2022 17:10
@bryanlb bryanlb force-pushed the bburkholder/parallel-segment-query branch from 9705487 to 84e5a75 Compare June 13, 2022 17:15
@bryanlb bryanlb requested a review from mansu June 13, 2022 21:25
@bryanlb bryanlb merged commit 9cc6373 into master Jun 13, 2022
@bryanlb bryanlb deleted the bburkholder/parallel-segment-query branch June 13, 2022 21:43
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.

3 participants