-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix and remove old benchmark code #50
Conversation
end | ||
end | ||
end | ||
end | ||
|
||
def compare_rewriting |
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.
I removed the rewriting benchmark because I can't find the Selma handlers that it's calling.
Benchmark.ips do |x| | ||
x.report("sanitize-document-#{label}") do |
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.
Shortened the output so that it no longer breaks the line:
original
Calculating -------------------------------------
sanitize-document-small
159.469 (± 0.6%) i/s - 798.000 in 5.004561s
selma-document-small 1.832k (± 1.7%) i/s - 9.180k in 5.012809s
revised
Calculating -------------------------------------
sanitize-small 140.647 (± 2.8%) i/s - 715.000 in 5.087293s
selma-small 1.924k (± 1.5%) i/s - 9.666k in 5.024335s
@gjtorikian just a small fix to get benchmarking working again. |
80eebb3
to
290a1c8
Compare
Accidentally removed a line during commit. Should work now. |
Hey, sorry for missing this! Life got in the way. I'll make time to review this this week! |
No problem! |
Hi! Thanks for caring enough to open this. I had to look through my backups to figure out what my intent here was, and found a file I never committed from 2022. HTML-Pipeline hadn't hit 3.0 yet, which is why the rewriters weren't found. Sorry about that! Anyway, I build on top of what you submitted, and having a working benchmark suite that shows off what I originally wanted it to. Thanks for your help! |
Oh very nice! Love seeing the comparison with nokolexbor, which I've run across in my investigations. |
No description provided.