Name each benchmark's winner and lint it - #243
Merged
Merged
Conversation
Each benchmark now says which report should win through its method names: `fastest`, `faster`, `fast`, then `slow`, `slower`, `slowest`. Every report is wrapped in one of these methods, so they all pay the same call cost, and the winner is listed first. Where a ranking could look odd, a one-line comment says why. - 11 files that ran code inline or used other names were converted. - start_with-vs-substring-==.rb reported code strings; each string now becomes a method, so its numbers are not comparable with older runs. - raise-vs-e2mmap.rb had two blocks that could not both claim `fast`; the custom exception block moved to raise-custom-vs-e2mmap.rb. - 21 files that already used `fast` listed a slower report first; only their report lines were reordered. - keys-include and values-include dropped `.shuffle`. It moved the looked-up item on every run, so the winner of values-include depended on where it landed. Both now look up the entry halfway through. Rankings follow the results from CI run 35927028164.
A fourth lint rule: in every Benchmark.ips block, exactly one report calls `fastest` (else `faster`, else `fast`), and it is the first report. The claim then comes from the code, not from README prose, and the results site can check it against every run. The lint job already runs this script and benchmarks-ok requires it, so the workflow does not change. CONTRIBUTING explains the naming with length-vs-size-vs-count.rb.
Finish the naming from Step 1 across all files, so each one reads top to bottom as its advice. - Every report now calls a rank-named method: avg, slow_dup, even_slower, slow2 and slow_1 to slow_5 are gone. Names follow the order in CI run 35927028164. - Names grow outward from the line between fast and slow, never skipping a step: the recommended side uses fast, faster, fastest, the other side slow, slower, slowest. The names give the order, not the size of the gap, which changes from one Ruby to the next. - Methods are defined in the same order as the reports. - proc-call-vs-yield.rb drops the "unused block" report: it never called the block, so it did less work than the others and won on 17 of 26 jobs. - One-line comments where a ranking depends on the Ruby or should tie (Range#member? is an alias of Range#include?). - inheritance-check.rb explains its two raise lines, and inject-symbol-vs-block.rb drops a require "rubygems" only Ruby 1.8 needed. - CONTRIBUTING's naming text and example follow the same rule.
Step 3 dropped its "unused block" report, but the README still showed it ranked first, which would give readers the wrong ranking. The new sample is from ruby_4.0: yield, then block + yield (1.23x slower), then block.call (1.41x slower), the same order as 22 of 26 jobs in CI run 35927028164.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fastest,faster,fast, thenslow,slower,slowest. The winner is listed first, so the claim is readable from the code, with no README prose needed.Changes
Step 1: name every benchmark's winner in its method names
slowonly means slower thanfast. Where a ranking could look odd, a one-line comment says why, for example inlength-vs-size-vs-count.rb: "Array#size is an alias of Array#length, so these two should tie."start_with-vs-substring-==.rbreported code strings. Each string now becomes a method, so its numbers are not comparable with older runs.raise-vs-e2mmap.rbhad two blocks that could not both claimfast. The custom exception block moved toraise-custom-vs-e2mmap.rb, and the README entry links and shows both.fastlisted a slower report first. Only their report lines were reordered.keys-include-vs-key.rbandvalues-include-vs-value.rbdropped.shuffle, which has been there since Add Hash#key? vs. Hash#keys.include? and Hash#value? vs. Hash#values.include? #154 with no reason given. It moved the looked-up item on every run, and in 9 of 20 test builds"zz"was a key, not a value. With the item at a fixed spot on ruby_3.4,value?won 1.67x at 10% in, andvalues.include?won 1.10x at 90% and 1.34x when missing. Both files now look up the entry halfway through.concatenation.rb's comments now give measured object counts per call (1, 1, 2, 2, 3 on Ruby 3.4). The old "3 object" comments were wrong for<<andconcat.Step 2: lint that each benchmark names one winner, first
.github/scripts/lint-benchmarks.rb: in everyBenchmark.ipsblock, exactly one report callsfastest(elsefaster, elsefast), and it is the first report.lintjob already runs this script, andbenchmarks-okrequires it.length-vs-size-vs-count.rb.Step 3: name every report by rank and define methods in that order
avg,slow_dup,even_slower,slow2andslow_1toslow_5are gone), in the order CI run 35927028164 measured.fast,faster,fasteston the recommended side,slow,slower,sloweston the other. They give the order, not the size of the gap, which changes from one Ruby to the next (for exampleString#match?is 0.96x to 2.07x of=~).proc-call-vs-yield.rbdrops the "unused block" report: it never called the block, so it did less work and won on 17 of 26 jobs. Without it,yieldwins on 22 of 26.Range#member?being an alias ofRange#include?.Step 4: rerun the
proc-call-vs-yield.rbREADME sampleTest plan
ruby_4.0, the command CI runs. Step 1 alone also passes the lint onmain.fastand a claimed report that is not first.mainexcept the split raise file and the dropped "unused block".start-string-checking-match-vs-start_with.rb(match?1.09x), happens only on 3.4:start_with?wins on 24 of 26 jobs in run 35927028164, and the file says so.key?wins everywhere (575x to 23363x).value?wins on JRuby (1.61x) and TruffleRuby (3.33x) and ties on the three CRubies..github/scripts/, so its own CI runs the full suite on all 26 jobs.Not covered
fastestand two reports callingfaststill passes, so "name them by rank" is a convention, not a check.