Skip to content

Add Duel performance regression guards - #3077

Open
sferik wants to merge 1 commit into
ruby-grape:masterfrom
sferik:add-duel
Open

sferik wants to merge 1 commit into
ruby-grape:masterfrom
sferik:add-duel

Conversation

@sferik

@sferik sferik commented Nov 10, 2026 •

Copy link
Copy Markdown

This adds Duel, a gem I wrote to catch performance regressions in CI. On every pull request it runs the benchmarks in duel/ against master and the branch side by side, in alternating worker processes, and fails the build if the branch is slower or allocates more. When something regresses, it profiles both versions with StackProf and says which method the extra time went to.

There are 39 benchmarks, and every one is a request sent through an API's Rack interface in memory, the way a server would hand it over, with no server or sockets involved. I went with whole requests rather than calling internal classes because that's what Grape users actually pay for, and because the router, the middleware stack and the endpoint have been tuned so carefully that most of the interesting work happens between them. The env for each request is built outside the measured time, since Grape and Rack write into the env and read rack.input, so reusing one would measure cached work. Each benchmark also checks its status and part of its body once before measuring, so none of them can quietly end up timing a 404.

grape-entity isn't in the development bundle, so the present benchmarks use a small representer class that present calls the same way it calls an Entity. Compiling a 200 route API took about 40 ms, which is longer than a benchmark should run, so the compile benchmark uses 20 routes and calls change! before each compile!.

A full comparison takes about 10 to 12 seconds here when nothing regressed.

The Duel job only reads. The comment on a pull request comes from a second workflow, perf-comment.yml, which runs none of the pull request's code, only duel comment from Duel's gem, and hides Duel's earlier comments as outdated so the newest one is the one in view. If a change is slower on purpose, a duel: accept label lets it through, and adding it runs the Duel job again.

@dblock dblock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice to see you here again @sferik! Interesting idea. I would want to hear an enthusiastic yes from @ericproulx who has been doing performance improvements left and right in grpe to merge it.

Nit, I'd refactor GHA workflows to be reusable so that they don't have to be copy-pasted all over, like we do here.

Thinking out loud, expanding CI to also check perf is something I'm 50/50 on. CI needs to get faster, not slower, to enable AI agents and benchmarks are as expensive as it gets.

Comment thread Rakefile Resolved
Duel (https://github.com/sferik/duel-ruby) runs the benchmarks in duel/
on the branch a pull request targets and on the pull request, side by
side, and fails the Perf workflow when the pull request is slower or
allocates more.

The 39 benchmarks send requests through an API's Rack interface in
memory: routing among 200 routes, nested namespaces, requirements and
wildcards, 404 and 405, params coercion and validation (query string,
form, JSON body, nested arrays, a custom validator, a 400), declared,
JSON formatting, present, content negotiation, error! and rescue_from,
callbacks and helpers, path, header and param versioning, mounting,
and compiling an API.

Comparing master with itself passed three times in a row. An extra dup
in Router#process_route failed 17 benchmarks on allocations, and
reading the attribute three times in CoerceValidator#validate_param!
failed 11 on instructions and CPU time.
@ericproulx

Copy link
Copy Markdown
Contributor

Thanks @sferik, this is a nice piece of work. The benchmarks cover the paths I've spent the most time on lately: the router's fast paths, the versioner cascade, declared and params.

@dblock I'm a yes on a trimmed-down version, details below.

What convinced me. The run on this PR changes nothing in lib/, so it's effectively an A/A test. All 39 benchmarks came out "same", and across the board it was 0.999 [0.995, 1.002]. Requiring the 99% interval to clear 1.02 keeps false alarms rare. The two checkouts also sit in same-length directories, which matters: I've measured identical code 5% apart under YJIT just because it lived at different paths.

The part I'd value most is the allocation counts. They're near-exact per request, so "this PR allocates more on the hot path" becomes a hard signal rather than a statistical one.

What it will and won't catch. Some benchmarks are noisy on shared runners: [0.839, 1.016] for the halting before, [0.887, 1.020] for the static route. So it'll flag a lost fast path or a per-request O(n) walk, not a 0.3% drift. That's the right trade for CI.

What I'd change before merging:

  1. Run it on pull requests only. The Duel job took 85s, against 42s for our slowest test job (25s median), so it would roughly double time-to-green. I'd drop the push, schedule and merge_group triggers. We squash-merge, so the push run mostly repeats the PR's, and with fail_on: :every_run a noisy push comparison turns master red. A paths: filter on lib/** and duel/** would skip the job entirely for docs and CI changes.
  2. Give it its own Gemfile. As it stands, duel (with fiddle) and stackprof land in every development bundle, including all 25 test jobs on 3.4 and 4.0. We just moved Danger out for the same reason (Use reusable Danger workflow with a separate Gemfile #3062). The Rakefile hook would then need a LoadError guard; as written, it already fails when the development group isn't installed.
  3. Pin the gem in perf-comment.yml. That job installs duel ~> 0.1.8 with a pull-requests: write token, so any future 0.1.x release would run with that token without review. An exact version, bumped on purpose, would be safer.
  4. A reusable workflow, as dblock suggested. If Duel shipped one, the way we consume Danger's, grape's side would be a few lines and fixes would arrive with Duel releases instead of by copy-paste. It would also replace the ~210 lines of YAML here.
  5. Consider YJIT. The workflow measures the interpreter, but Rails turns YJIT on by default, and some Grape changes move differently under it. If there's budget for only one mode, I'd measure YJIT.

Smaller things: the CHANGELOG entry is missing. It would also help to add a line on how duel/ relates to benchmark/, which already has grape_on_rack and version_throughput, so contributors know where a new scenario goes.

@sferik

sferik commented Nov 13, 2026

Copy link
Copy Markdown
Author

@ericproulx Thank you for the thoughtful review. I will work on making these changes and push an updated review shortly.

Nit, I'd refactor GHA workflows to be reusable so that they don't have to be copy-pasted all over, like we do here.

I have added this feature on Duel’s main branch, which I’m hoping to release tomorrow. I’ll be sure to update the PR to use that as well.

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