-
Notifications
You must be signed in to change notification settings - Fork 146
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
add benchmark script #2359
add benchmark script #2359
Conversation
a4c3899
to
4b56eef
Compare
Thanks @exterm! Would you mind adding it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just marking as "request changes"
4b56eef
to
9ad7968
Compare
Done and I also added |
9ad7968
to
f09ace2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
||
Benchmark.ips do |x| | ||
x.report("Parser::CurrentRuby") { Parser::CurrentRuby.parse_file(filepath) } | ||
x.report("Parser::Prism") { Prism.parse_file(filepath) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be labeled just Prism
as there is no Parser::Prism
(it confused me for the results in the PR description).
I was about to make a PR but @kddnewton already fixed it in 40059d3 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah dang, that's copy-pasta. Thanks for the note, Benoit
Similar to what @kddnewton did in parser-prism.
I needed an easy way to compare performance of the different parser options. I think it would make sense for it to live in this repo for now.
These are the results from a run on my local machine:
Prism 0.23.0
Prism 0.24.0