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

Remodel boolean queries to hold last query #172

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

valencik
Copy link
Collaborator

No description provided.

@valencik valencik self-assigned this Nov 26, 2024
@valencik
Copy link
Collaborator Author

Benchmarking b2c5a20 with 89c8790 cherry-picked ontop with my AMD Ryzen 7 3800X 8-Core Processor.

Benchmark                                              Mode  Cnt     Score    Error   Units
MapLastTermBenchmark.associativityQueriesMapLastTerm  thrpt   10     8.191 ±  0.171  ops/ms
MapLastTermBenchmark.partialQueriesMapLastTerm        thrpt   10     0.626 ±  0.042  ops/ms
QueryParserBenchmark.associativityQueriesParse        thrpt   10     8.441 ±  0.341  ops/ms
QueryParserBenchmark.orQueries1000Parse               thrpt   10     0.613 ±  0.048  ops/ms
QueryParserBenchmark.orQueries10Parse                 thrpt   10    57.437 ±  0.292  ops/ms
QueryPrinterBenchmark.orQueries1000Print              thrpt   10    28.264 ±  0.396  ops/ms
QueryPrinterBenchmark.orQueries10Print                thrpt   10  4717.728 ± 46.392  ops/ms
QueryPrinterBenchmark.termQueriesPrint                thrpt   10  1082.307 ± 32.293  ops/ms

Compared to "two-nodes" 9efbfed

Benchmark                                              Mode  Cnt     Score     Error   Units
MapLastTermBenchmark.associativityQueriesMapLastTerm  thrpt   10     8.220 ±   0.086  ops/ms
MapLastTermBenchmark.partialQueriesMapLastTerm        thrpt   10     0.592 ±   0.016  ops/ms
QueryParserBenchmark.associativityQueriesParse        thrpt   10     8.396 ±   0.456  ops/ms
QueryParserBenchmark.orQueries1000Parse               thrpt   10     0.608 ±   0.041  ops/ms
QueryParserBenchmark.orQueries10Parse                 thrpt   10    65.001 ±   3.273  ops/ms
QueryPrinterBenchmark.orQueries1000Print              thrpt   10    25.832 ±   4.318  ops/ms
QueryPrinterBenchmark.orQueries10Print                thrpt   10  4424.268 ± 237.150  ops/ms
QueryPrinterBenchmark.termQueriesPrint                thrpt   10  1115.661 ±   7.945  ops/ms

Compared to "main" (from #171 (comment))

Benchmark                                              Mode  Cnt     Score    Error   Units
MapLastTermBenchmark.associativityQueriesMapLastTerm  thrpt   10     8.221 ±  0.234  ops/ms
MapLastTermBenchmark.partialQueriesMapLastTerm        thrpt   10     0.703 ±  0.023  ops/ms
QueryParserBenchmark.associativityQueriesParse        thrpt   10     9.080 ±  0.144  ops/ms
QueryParserBenchmark.orQueries1000Parse               thrpt   10     0.650 ±  0.022  ops/ms
QueryParserBenchmark.orQueries10Parse                 thrpt   10    71.972 ±  2.887  ops/ms
QueryPrinterBenchmark.orQueries1000Print              thrpt   10    28.541 ±  0.392  ops/ms
QueryPrinterBenchmark.orQueries10Print                thrpt   10  4664.162 ± 81.206  ops/ms
QueryPrinterBenchmark.termQueriesPrint                thrpt   10  1091.093 ± 16.845  ops/ms

@valencik
Copy link
Collaborator Author

My current takeaway is that neither the changes here or in #170 are making the parser any faster, just a bit slower.
And while #170 is definitely aimed at making the model more correct, this PR was hopefully going to make mapLastTerm faster. But it's totally dominated by the parsing time anyways.
We also rewrite associateOps in this PR, and it's not immediately obvious that it's a whole lot simpler here.

Base automatically changed from two-nodes to main November 28, 2024 00:29
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