Skip to content

Add benchmarks using cargo bench / criterion #190

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

Merged
merged 3 commits into from
Jun 7, 2020

Conversation

Dandandan
Copy link
Contributor

Adds two simple benchmarks, using cargo bench and criterion.

On my machine:

sqlparser-rs parsing benchmark/sqlparser::select                        
                        time:   [13.409 us 13.797 us 14.171 us]

sqlparser-rs parsing benchmark/sqlparser::with_select                        
                        time:   [69.353 us 71.665 us 74.025 us]

@nickolay
Copy link
Contributor

nickolay commented Jun 7, 2020

Thanks! (This fixes #188.) As I feared, this slows down the clean build (e.g. cargo test) considerably - 6 wall-clock seconds / 23 user CPU vs 23/120 seconds - a 4x slowdown, this is too large a price for something rarely used.

rust-lang/cargo#1596 (comment) describes a possible solution, would you be willing to do that? Alternatively we could keep this on a branch of its own and include instructions on cherry-picking it in the README on master.

@Dandandan
Copy link
Contributor Author

That would be a nice solution. I'll be looking into that!

@Dandandan
Copy link
Contributor Author

I moved them to sqlparser_bench

@nickolay
Copy link
Contributor

nickolay commented Jun 7, 2020

Awesome, thank you!

@nickolay nickolay closed this Jun 7, 2020
@nickolay nickolay reopened this Jun 7, 2020
@nickolay nickolay merged commit 6e6fae7 into apache:master Jun 7, 2020
@Dandandan Dandandan mentioned this pull request Jun 7, 2020
@@ -0,0 +1,7 @@
#[cfg(test)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intended to be committed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, is the result of running cargo new --lib, didn't notice that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @maxcountryman! I removed this file from master.

nickolay referenced this pull request Jun 10, 2020
- Update CHANGELOG
- Update `.gitignore` for the build directory of the benchmark crate
- Remove src/lib from the recently added benchmark crate per
  https://github.com/andygrove/sqlparser-rs/pull/190#pullrequestreview-425835379
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