From e3a47bfca1757a0f264947f9ccd4146cb103e7ca Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Sun, 25 Oct 2026 10:25:29 -0700 Subject: [PATCH 1/3] Add Duel performance regression guards Benchmark the hot paths with Duel, so that a pull request that makes them slower fails CI: urlencoding small, large, nested and multibyte params, building multipart bodies with text fields and in-memory files, and reading bodies with to_s, in 16 KB chunks as http.rb streams them, rewind and content length. Every multipart body uses a fixed boundary. Duel requires Ruby 3.4 or later, so the Gemfile adds it only there and the Rake task is loaded only when Duel is. The Perf workflow compares each pull request with master, each push with the commit before it, and the last release on a schedule. RuboCop now checks .duel.rb too. The Duel job only reads, and a Comment job of its own, which checks nothing out and runs none of the branch's code, comments on the pull request with the report. A change to only the benchmarks, .duel.rb or the Perf workflow is compared too. --- .duel.rb | 17 +++++++ .github/workflows/perf.yml | 95 ++++++++++++++++++++++++++++++++++++++ .rubocop.yml | 6 +++ Gemfile | 6 +++ README.md | 1 + Rakefile | 7 +++ duel/duel_helper.rb | 54 ++++++++++++++++++++++ duel/multipart_duel.rb | 95 ++++++++++++++++++++++++++++++++++++++ duel/urlencoded_duel.rb | 33 +++++++++++++ 9 files changed, 314 insertions(+) create mode 100644 .duel.rb create mode 100644 .github/workflows/perf.yml create mode 100644 duel/duel_helper.rb create mode 100644 duel/multipart_duel.rb create mode 100644 duel/urlencoded_duel.rb diff --git a/.duel.rb b/.duel.rb new file mode 100644 index 0000000..1578055 --- /dev/null +++ b/.duel.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# Duel's configuration: https://github.com/sferik/duel-ruby/blob/main/docs/configuration.md +# +# Duel follows conventions, so set only what differs: benchmarks are in +# duel/**/*_duel.rb, each version's lib/ is first on the load path, a +# change is compared when it touches lib/ or ext/, and releases are +# tagged v*. +Duel.configure do |c| + # The library each version's workers require (from the gemspec, unless + # this says otherwise). + # c.library = "http/form_data" + + # Compare a change to the benchmarks, this file or the Perf workflow + # too, not only to lib/ and ext/. + c.watch += %w[duel .duel.rb .github/workflows/perf.yml] +end diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml new file mode 100644 index 0000000..fccc7be --- /dev/null +++ b/.github/workflows/perf.yml @@ -0,0 +1,95 @@ +name: Perf + +on: + pull_request: + push: + branches: [master] + schedule: + - cron: "0 4 * * *" + workflow_dispatch: + +# The Duel job runs the branch's code (its benchmarks too), so it only +# reads. Commenting is the Comment job's, which runs none of it. +permissions: + contents: read + +env: + BUNDLE_WITHOUT: "sig" + +jobs: + duel: + name: Duel + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 # the baseline comes from history, tags included + persist-credentials: false + # (And whatever else your tests need set up: packages, services.) + - uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.4" + bundler-cache: true + # Against the main branch for a pull request, what was there before + # for a push, and the last release on a schedule (slow creep). Duel + # adds its report to the job's summary, and writes it to duel.md for + # the comment. + - name: Compare + env: + DUEL_BASE: ${{ github.event_name == 'schedule' && 'last-release' || github.event_name == 'push' && github.event.before || '' }} + # (Run by hand, it compares anyway: with nothing changed, the code + # with itself, which checks the setup.) + DUEL_FORCE: ${{ github.event_name == 'workflow_dispatch' && '1' || '' }} + run: | + # (A branch's first push, or one that rewrote history, has no + # commit before it here to compare against.) + if [ "$GITHUB_EVENT_NAME" = push ] && ! git cat-file -e "$DUEL_BASE^{commit}" 2>/dev/null; then + echo "::notice::No commit before this push to compare against" + exit 0 + fi + bundle exec duel --markdown "$RUNNER_TEMP/duel.md" + # The report, for the Comment job. + - name: Keep the report + if: ${{ !cancelled() && github.event_name == 'pull_request' }} + uses: actions/upload-artifact@v7 + with: + name: duel-report + path: ${{ runner.temp }}/duel.md + if-no-files-found: ignore # none when Duel didn't get to compare + retention-days: 1 + + # The report, as a comment on the pull request: one comment, updated on + # every push. This job checks nothing out and runs none of the branch's + # code, so it alone can write. (A pull request from a fork gets no + # comment, as its token can't write one: the Duel job's summary has the + # report.) + comment: + name: Comment + needs: duel + if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository }} + runs-on: ubuntu-latest + permissions: + pull-requests: write # to comment on the pull request with the report + steps: + - name: Get the report + id: report + continue-on-error: true # there's none when the Duel job failed before comparing + uses: actions/download-artifact@v8 + with: + name: duel-report + path: ${{ runner.temp }}/report + - name: Comment on the pull request + if: steps.report.outcome == 'success' + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + run: | + marker="" + { echo "$marker"; cat "$RUNNER_TEMP/report/duel.md"; } > "$RUNNER_TEMP/comment.md" + ids=$(gh api --paginate "repos/$GITHUB_REPOSITORY/issues/$PR/comments" \ + --jq ".[] | select(.user.login == \"github-actions[bot]\" and (.body | startswith(\"$marker\"))) | .id") + if [ -n "$ids" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/${ids%%$'\n'*}" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null + else + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR/comments" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null + fi diff --git a/.rubocop.yml b/.rubocop.yml index d5400c9..609907e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,7 +8,13 @@ inherit_from: - .rubocop/metrics.yml - .rubocop/style.yml +inherit_mode: + merge: + - Include + AllCops: + Include: + - .duel.rb DisplayCopNames: true DisplayStyleGuide: true NewCops: enable diff --git a/Gemfile b/Gemfile index 954c956..1cbf800 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,12 @@ source "https://rubygems.org" gem "rake" +group :development do + # Duel requires Ruby 3.4 or later + gem "duel" if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4") + gem "stackprof", platforms: :mri +end + group :test do gem "minitest", "~> 6.0" diff --git a/README.md b/README.md index 5e25cb3..aef0af0 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ [![Type Check](https://github.com/httprb/form_data/actions/workflows/typecheck.yml/badge.svg)](https://github.com/httprb/form_data/actions/workflows/typecheck.yml) [![Mutant](https://github.com/httprb/form_data/actions/workflows/mutant.yml/badge.svg)](https://github.com/httprb/form_data/actions/workflows/mutant.yml) [![Docs](https://github.com/httprb/form_data/actions/workflows/docs.yml/badge.svg)](https://github.com/httprb/form_data/actions/workflows/docs.yml) +[![Duel](https://raw.githubusercontent.com/sferik/duel-ruby/main/assets/badge.svg)](https://github.com/sferik/duel-ruby) Utility-belt to build form data request bodies. diff --git a/Rakefile b/Rakefile index d102d3d..8f6037a 100644 --- a/Rakefile +++ b/Rakefile @@ -41,3 +41,10 @@ task :mutant do end task default: %i[test rubocop verify_measurements steep] + +begin + require "duel/rake_task" + Duel::RakeTask.new +rescue LoadError + # Duel requires Ruby 3.4 or later +end diff --git a/duel/duel_helper.rb b/duel/duel_helper.rb new file mode 100644 index 0000000..fa4199e --- /dev/null +++ b/duel/duel_helper.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require "pathname" # (FormData::File checks for a Pathname, but doesn't require it) +require "stringio" + +# A fixed boundary, so that every iteration (and both versions) builds +# the same body. Multipart.generate_boundary would make a random one. +BOUNDARY = ("-" * 21) + ("0123456789abcdef" * 3)[0, 42] + +# http.rb streams a body with IO.copy_stream, which reads it 16 KB at a +# time into one buffer (HTTP::Connection::BUFFER_SIZE). +CHUNK_SIZE = 16_384 + +# A login form's fields. +def small_params + { "username" => "ixti", "password" => "s3cr3t p@ss", "remember" => "1" } +end + +# A form of `count` fields, with values that need escaping. +def large_params(count) + Array.new(count) { |i| ["field_#{i}", "value #{i} & more/stuff=#{'x' * (i % 17)}"] }.to_h +end + +# Nested hashes and arrays, as Rails-style params. +NESTED_PARAMS = { + "user" => { + "name" => "Aleksey Zapparov", + "email" => "ixti@example.com", + "address" => { "street" => "1 Main St", "city" => "Springfield", "zip" => "12345" }, + "roles" => %w[admin editor viewer], + "preferences" => { "theme" => "dark", "languages" => %w[en ru uk], "notify" => nil } + }, + "items" => Array.new(20) { |i| { "id" => i.to_s, "qty" => (i * 3).to_s, "tags" => %w[a b] } }, + "q" => "search terms with spaces & symbols" +}.freeze + +# `size` bytes of binary data, the same every time. +def binary_data(size) + Random.new(42).bytes(size) +end + +# A file part from memory, with its filename and content type given, so +# that its name doesn't come from the IO's object_id. +def file_part(data, filename: "upload.bin", content_type: "application/octet-stream") + HTTP::FormData::File.new(StringIO.new(data), filename: filename, content_type: content_type) +end + +# Reads the whole body as http.rb does, 16 KB at a time into one buffer. +def read_in_chunks(body, buffer) + body.rewind + bytes = 0 + bytes += buffer.bytesize while body.read(CHUNK_SIZE, buffer) + bytes +end diff --git a/duel/multipart_duel.rb b/duel/multipart_duel.rb new file mode 100644 index 0000000..3a68ba4 --- /dev/null +++ b/duel/multipart_duel.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +duel "multipart: build a few text fields" do + once { @params = small_params } + HTTP::FormData::Multipart.new(@params, boundary: BOUNDARY) +end + +duel "multipart: build 200 text fields" do + once { @params = large_params(200) } + HTTP::FormData::Multipart.new(@params, boundary: BOUNDARY) +end + +duel "multipart: build with an array of values" do + once { @params = { "name" => "ixti", "tags" => Array.new(20) { |i| "tag#{i}" } } } + HTTP::FormData::Multipart.new(@params, boundary: BOUNDARY) +end + +duel "multipart: build with a file" do |n| + once { @data = binary_data(4096) } + files = untimed { Array.new(n) { file_part(@data, filename: "avatar.png", content_type: "image/png") } } + files.each { |file| HTTP::FormData::Multipart.new({ "username" => "ixti", "avatar" => file }, boundary: BOUNDARY) } +end + +duel "multipart: build with 20 files" do + once do + @params = Array.new(20) { |i| ["files[]", HTTP::FormData::Part.new("x" * 512, filename: "f#{i}.txt", content_type: "text/plain")] } + end + HTTP::FormData::Multipart.new(@params, boundary: BOUNDARY) +end + +duel "multipart: to_s a few text fields" do + once { @form = HTTP::FormData::Multipart.new(small_params, boundary: BOUNDARY) } + @form.to_s +end + +duel "multipart: to_s 200 text fields" do + once { @form = HTTP::FormData::Multipart.new(large_params(200), boundary: BOUNDARY) } + @form.to_s +end + +duel "multipart: to_s a 16 KB file" do + once { @form = HTTP::FormData::Multipart.new({ "file" => file_part(binary_data(16_384)) }, boundary: BOUNDARY) } + @form.to_s +end + +duel "multipart: read a 64 KB file in 16 KB chunks" do + once do + @form = HTTP::FormData::Multipart.new({ "name" => "ixti", "file" => file_part(binary_data(65_536)) }, + boundary: BOUNDARY) + @buffer = String.new(capacity: CHUNK_SIZE) + end + read_in_chunks(@form, @buffer) +end + +duel "multipart: read 200 fields in 16 KB chunks" do + once do + @form = HTTP::FormData::Multipart.new(large_params(200), boundary: BOUNDARY) + @buffer = String.new(capacity: CHUNK_SIZE) + end + read_in_chunks(@form, @buffer) +end + +duel "multipart: read in small chunks" do + once do + @form = HTTP::FormData::Multipart.new(small_params.merge("file" => file_part(binary_data(4096))), + boundary: BOUNDARY) + @buffer = String.new(capacity: 64) + end + @form.rewind + nil while @form.read(64, @buffer) +end + +duel "multipart: rewind 200 fields" do + once do + @form = HTTP::FormData::Multipart.new(large_params(200), boundary: BOUNDARY) + @form.to_s + end + @form.rewind +end + +duel "multipart: content length of 200 fields" do |n| + once { @params = large_params(200) } + forms = untimed { Array.new(n) { HTTP::FormData::Multipart.new(@params, boundary: BOUNDARY) } } + forms.each(&:content_length) +end + +duel "multipart: content length with a file" do |n| + once { @data = binary_data(4096) } + forms = untimed do + Array.new(n) { HTTP::FormData::Multipart.new({ "name" => "ixti", "file" => file_part(@data) }, boundary: BOUNDARY) } + end + forms.each(&:content_length) +end diff --git a/duel/urlencoded_duel.rb b/duel/urlencoded_duel.rb new file mode 100644 index 0000000..eb5413b --- /dev/null +++ b/duel/urlencoded_duel.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +duel "urlencode: a small form" do + once { @params = small_params } + HTTP::FormData.create(@params) +end + +duel "urlencode: a large form of 200 fields" do + once { @params = large_params(200) } + HTTP::FormData.create(@params) +end + +duel "urlencode: nested hashes and arrays" do + once { @params = NESTED_PARAMS } + HTTP::FormData.create(@params) +end + +duel "urlencode: pairs with repeated keys" do + once { @pairs = Array.new(50) { |i| ["tag", "tag #{i}"] } + [%w[page 2], ["flag", nil]] } + HTTP::FormData.create(@pairs) +end + +duel "urlencode: values with multibyte characters" do + once { @params = { "name" => "Алексей Запаров", "city" => "東京都", "note" => "café ☕ — naïve résumé" } } + HTTP::FormData.create(@params) +end + +duel "urlencode: read a large body" do + once { @form = HTTP::FormData.create(large_params(200)) } + @form.to_s +end From fe9c80c97506679614b671409ba90a5b2dd69299 Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Mon, 26 Oct 2026 09:04:56 -0700 Subject: [PATCH 2/3] Update Duel to 0.1.1 The Perf workflow now comments on a pull request only when Duel fails, as c.comment says by default, and posts a new comment instead of editing the last one. Duel 0.1.1 watches the benchmarks and .duel.rb itself, so .duel.rb only adds the workflow. --- .duel.rb | 5 ++--- .github/workflows/perf.yml | 41 ++++++++++++++++++++++++-------------- Gemfile | 2 +- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/.duel.rb b/.duel.rb index 1578055..a6b105f 100644 --- a/.duel.rb +++ b/.duel.rb @@ -11,7 +11,6 @@ # this says otherwise). # c.library = "http/form_data" - # Compare a change to the benchmarks, this file or the Perf workflow - # too, not only to lib/ and ext/. - c.watch += %w[duel .duel.rb .github/workflows/perf.yml] + # Compare a change to only the Perf workflow too. + c.watch += %w[.github/workflows/perf.yml] end diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml index fccc7be..c4d4a45 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -20,6 +20,11 @@ jobs: duel: name: Duel runs-on: ubuntu-latest + # Whether the Comment job comments (post, change or none), and the + # run's status (passed, failed or neutral), from Duel. + outputs: + comment: ${{ steps.compare.outputs.comment }} + status: ${{ steps.compare.outputs.status }} steps: - uses: actions/checkout@v7 with: @@ -32,9 +37,11 @@ jobs: bundler-cache: true # Against the main branch for a pull request, what was there before # for a push, and the last release on a schedule (slow creep). Duel - # adds its report to the job's summary, and writes it to duel.md for - # the comment. + # adds its report to the job's summary, writes it to duel.md for the + # comment, and sets the step's outputs to say whether to comment, as + # c.comment in .duel.rb says, whatever the outcome. - name: Compare + id: compare env: DUEL_BASE: ${{ github.event_name == 'schedule' && 'last-release' || github.event_name == 'push' && github.event.before || '' }} # (Run by hand, it compares anyway: with nothing changed, the code @@ -58,15 +65,17 @@ jobs: if-no-files-found: ignore # none when Duel didn't get to compare retention-days: 1 - # The report, as a comment on the pull request: one comment, updated on - # every push. This job checks nothing out and runs none of the branch's - # code, so it alone can write. (A pull request from a fork gets no - # comment, as its token can't write one: the Duel job's summary has the - # report.) + # The report, as a new comment on the pull request, when Duel says to: + # always (post), or only when its last comment's status differs from + # this run's (change), so the latest comment is never stale. It never + # edits or deletes a comment. This job checks nothing out and runs none + # of the branch's code, so it alone can write. (A pull request from a + # fork gets no comment, as its token can't write one: the Duel job's + # summary has the report.) comment: name: Comment needs: duel - if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository }} + if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && (needs.duel.outputs.comment == 'post' || needs.duel.outputs.comment == 'change') }} runs-on: ubuntu-latest permissions: pull-requests: write # to comment on the pull request with the report @@ -83,13 +92,15 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} + COMMENT: ${{ needs.duel.outputs.comment }} + STATUS: ${{ needs.duel.outputs.status }} run: | - marker="" - { echo "$marker"; cat "$RUNNER_TEMP/report/duel.md"; } > "$RUNNER_TEMP/comment.md" - ids=$(gh api --paginate "repos/$GITHUB_REPOSITORY/issues/$PR/comments" \ - --jq ".[] | select(.user.login == \"github-actions[bot]\" and (.body | startswith(\"$marker\"))) | .id") - if [ -n "$ids" ]; then - gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/${ids%%$'\n'*}" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null - else + marker="\")" | tail -n 1) + if [ "$COMMENT" = post ] || { [ -n "$last" ] && [ "$last" != "$marker status=$STATUS -->" ]; }; then + { echo "$marker status=$STATUS -->"; cat "$RUNNER_TEMP/report/duel.md"; } > "$RUNNER_TEMP/comment.md" gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR/comments" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null fi diff --git a/Gemfile b/Gemfile index 1cbf800..6f050a8 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ gem "rake" group :development do # Duel requires Ruby 3.4 or later - gem "duel" if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4") + gem "duel", ">= 0.1.1" if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4") gem "stackprof", platforms: :mri end From 9b8155261566bd585106992a210da009f86bbe33 Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Mon, 2 Nov 2026 16:22:49 -0800 Subject: [PATCH 3/3] Update Duel to 0.1.2 Duel 0.1.2 asks a project to regenerate its workflows with `duel init`. The Perf workflow loses its Comment job to a new Perf comment workflow, which runs once Perf has, with a token that can write, so a pull request from a fork gets a comment too. The regenerated Perf workflow also runs for a merge queue, cancels a pull request's stale runs, and no longer checks for a missing commit before a push, which Duel now does itself. .duel.rb takes up the new settings. A push to master is compared with the last commit Duel passed (duel/passed/master), so small slowdowns add up until one fails. A regression there warns, and the workflow comments on the commit, instead of failing the run. Pull requests still fail, unless they have the "duel: accept" label. --- .duel.rb | 18 ++++ .github/workflows/perf-comment.yml | 59 ++++++++++++ .github/workflows/perf.yml | 139 +++++++++++++++++++++-------- Gemfile | 2 +- 4 files changed, 179 insertions(+), 39 deletions(-) create mode 100644 .github/workflows/perf-comment.yml diff --git a/.duel.rb b/.duel.rb index a6b105f..5dda35b 100644 --- a/.duel.rb +++ b/.duel.rb @@ -13,4 +13,22 @@ # Compare a change to only the Perf workflow too. c.watch += %w[.github/workflows/perf.yml] + + # A regression fails a pull request (or a merge queue), but in a push + # to the master branch, the schedule, or a run by hand, it only warns, + # and the workflow comments on the pushed commit with the report, + # which notifies its author. + c.fail_on = :pull_requests + + # A pull request with this label passes with a regression, which the + # report says the label accepted (after adding the label, run the + # workflow again). + c.accept_label = "duel: accept" + + # A push is compared with the last commit Duel passed, which a branch + # marks (duel/passed/master for master), not the commit before it, so + # slowdowns too small for any one push to fail add up until they fail + # one. To accept a regression after it's pushed, move that branch: + # git push origin master:duel/passed/master. + c.push_base = :last_passed end diff --git a/.github/workflows/perf-comment.yml b/.github/workflows/perf-comment.yml new file mode 100644 index 0000000..9034d54 --- /dev/null +++ b/.github/workflows/perf-comment.yml @@ -0,0 +1,59 @@ +name: Perf comment + +# Duel's report, as a new comment on the pull request, when Duel says to: +# always (post), or only when its last comment's status differs from the +# run's (change), so the latest comment is never stale. It never edits or +# deletes a comment. It runs once the Perf workflow has, with this +# repository's own workflow and a token that can write, so a pull request +# from a fork gets a comment too. It checks nothing out and runs none of +# the pull request's code, and of what the Duel job kept (where that code +# ran), it trusts only what that job could have said: the pull request's +# number, if it names the pull request the run compared, and the comment +# and the status, if they're ones Duel says. (The workflow must be on the +# default branch before it runs at all.) +on: + workflow_run: + workflows: ["Perf"] + types: [completed] + +permissions: {} + +jobs: + comment: + name: Comment + if: ${{ github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion != 'cancelled' }} + runs-on: ubuntu-latest + permissions: + actions: read # to get the report from the Perf run + pull-requests: write # to comment on the pull request with the report + steps: + - name: Get the report + id: report + continue-on-error: true # there's none when the Duel job failed before comparing + uses: actions/download-artifact@v8 + with: + name: duel-report + path: ${{ runner.temp }}/report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + - name: Comment on the pull request + if: steps.report.outcome == 'success' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + said() { grep -xE "$1=($2)" "$RUNNER_TEMP/report/comment.env" | tail -n 1 | cut -d= -f2; } + PR=$(said pr '[0-9]+') + COMMENT=$(said comment 'post|change') + STATUS=$(said status 'passed|failed|neutral') + [ -n "$PR" ] && [ -n "$COMMENT" ] && [ -n "$STATUS" ] || exit 0 + [ "$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR" --jq .head.sha)" = "$HEAD_SHA" ] || exit 0 + marker="\")" | tail -n 1) + if [ "$COMMENT" = post ] || { [ -n "$last" ] && [ "$last" != "$marker status=$STATUS -->" ]; }; then + { echo "$marker status=$STATUS -->"; cat "$RUNNER_TEMP/report/duel.md"; } > "$RUNNER_TEMP/comment.md" + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR/comments" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null + fi diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml index c4d4a45..02f71c6 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -2,16 +2,27 @@ name: Perf on: pull_request: + merge_group: # (a merge queue's) push: - branches: [master] + branches: [master] # (and any other branch to guard, a stable one, say) schedule: - cron: "0 4 * * *" workflow_dispatch: +# A pull request's newer push cancels its run, if that's still going, as +# only its newest commit's result counts. Every other run (a push's, say) +# runs to the end, so that each push is compared. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + # The Duel job runs the branch's code (its benchmarks too), so it only -# reads. Commenting is the Comment job's, which runs none of it. +# reads. Commenting on a commit is the Comment on the commit job's, which +# runs none of it, and commenting on a pull request is the Perf comment +# workflow's. permissions: contents: read + pull-requests: read # for the pull request's labels (see c.accept_label) env: BUNDLE_WITHOUT: "sig" @@ -20,11 +31,13 @@ jobs: duel: name: Duel runs-on: ubuntu-latest - # Whether the Comment job comments (post, change or none), and the - # run's status (passed, failed or neutral), from Duel. + # Whether to comment (post, change or none), the run's status (passed, + # failed or neutral), and whether to mark a push passed (true, or + # nothing), from Duel. outputs: comment: ${{ steps.compare.outputs.comment }} status: ${{ steps.compare.outputs.status }} + passed: ${{ steps.compare.outputs.passed }} steps: - uses: actions/checkout@v7 with: @@ -35,50 +48,80 @@ jobs: with: ruby-version: "3.4" bundler-cache: true - # Against the main branch for a pull request, what was there before - # for a push, and the last release on a schedule (slow creep). Duel - # adds its report to the job's summary, writes it to duel.md for the - # comment, and sets the step's outputs to say whether to comment, as - # c.comment in .duel.rb says, whatever the outcome. + # The labels of the pull request (or for a push, of the one that + # merged it), for c.accept_label in .duel.rb, as a JSON array. They're + # read as the job runs, so after adding the label, run it again. + - name: Find the pull request's labels + id: labels + if: github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'push' + continue-on-error: true # (without them, nothing's accepted) + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + QUEUED: ${{ github.event.merge_group.head_ref }} + run: | + # (In a merge queue, the last pull request queued, which the queue's + # branch names: gh-readonly-queue/main/pr-123-.) + if [ -n "$QUEUED" ]; then + PR=${QUEUED##*/pr-} + PR=${PR%%-*} + fi + if [ -n "$PR" ]; then + labels=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR/labels" --jq '[.[].name]') + else + labels=$(gh api "repos/$GITHUB_REPOSITORY/commits/$GITHUB_SHA/pulls" --jq '[.[0].labels[]?.name]') + fi + echo "labels=$labels" >> "$GITHUB_OUTPUT" + # Against the branch it targets for a pull request, the queue's base + # for a merge queue, what was there before for a push (or as + # c.push_base in .duel.rb says, the last commit Duel passed), and the + # last release on a schedule (slow creep). Duel adds its report to + # the job's summary, writes it to duel.md for the comment, and sets + # the step's outputs to say whether to comment, as c.comment in + # .duel.rb says, whatever the outcome. - name: Compare id: compare env: - DUEL_BASE: ${{ github.event_name == 'schedule' && 'last-release' || github.event_name == 'push' && github.event.before || '' }} + DUEL_BASE: ${{ github.event_name == 'schedule' && 'last-release' || github.event_name == 'push' && github.event.before || github.event_name == 'merge_group' && github.event.merge_group.base_sha || '' }} # (Run by hand, it compares anyway: with nothing changed, the code # with itself, which checks the setup.) DUEL_FORCE: ${{ github.event_name == 'workflow_dispatch' && '1' || '' }} + DUEL_LABELS: ${{ steps.labels.outputs.labels }} + run: bundle exec duel --markdown "$RUNNER_TEMP/duel/duel.md" + # For a pull request, what Duel said, for the Perf comment workflow, + # which can't see this job's outputs. + - name: Say whether to comment + if: ${{ !cancelled() && github.event_name == 'pull_request' }} + env: + PR: ${{ github.event.pull_request.number }} + COMMENT: ${{ steps.compare.outputs.comment }} + STATUS: ${{ steps.compare.outputs.status }} run: | - # (A branch's first push, or one that rewrote history, has no - # commit before it here to compare against.) - if [ "$GITHUB_EVENT_NAME" = push ] && ! git cat-file -e "$DUEL_BASE^{commit}" 2>/dev/null; then - echo "::notice::No commit before this push to compare against" - exit 0 - fi - bundle exec duel --markdown "$RUNNER_TEMP/duel.md" - # The report, for the Comment job. + mkdir -p "$RUNNER_TEMP/duel" + printf 'pr=%s\ncomment=%s\nstatus=%s\n' "$PR" "$COMMENT" "$STATUS" > "$RUNNER_TEMP/duel/comment.env" + # The report, for the comments. - name: Keep the report - if: ${{ !cancelled() && github.event_name == 'pull_request' }} + if: ${{ !cancelled() && (github.event_name == 'pull_request' || github.event_name == 'push') }} uses: actions/upload-artifact@v7 with: name: duel-report - path: ${{ runner.temp }}/duel.md + path: ${{ runner.temp }}/duel/ if-no-files-found: ignore # none when Duel didn't get to compare retention-days: 1 - # The report, as a new comment on the pull request, when Duel says to: - # always (post), or only when its last comment's status differs from - # this run's (change), so the latest comment is never stale. It never - # edits or deletes a comment. This job checks nothing out and runs none - # of the branch's code, so it alone can write. (A pull request from a - # fork gets no comment, as its token can't write one: the Duel job's - # summary has the report.) - comment: - name: Comment + # The report, as a new comment on a pushed commit, which notifies its + # author, when Duel says to, as for a pull request (see the Perf comment + # workflow). It does only where a regression in a push passes, with + # c.fail_on = :pull_requests in .duel.rb, so the comment says what the + # check doesn't. Its last comment is the last one on any commit. This + # job checks nothing out and runs none of the code. + commit-comment: + name: Comment on the commit needs: duel - if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && (needs.duel.outputs.comment == 'post' || needs.duel.outputs.comment == 'change') }} + if: ${{ !cancelled() && github.event_name == 'push' && (needs.duel.outputs.comment == 'post' || needs.duel.outputs.comment == 'change') }} runs-on: ubuntu-latest permissions: - pull-requests: write # to comment on the pull request with the report + contents: write # to comment on the commit with the report steps: - name: Get the report id: report @@ -87,20 +130,40 @@ jobs: with: name: duel-report path: ${{ runner.temp }}/report - - name: Comment on the pull request + - name: Comment on the commit if: steps.report.outcome == 'success' env: GH_TOKEN: ${{ github.token }} - PR: ${{ github.event.pull_request.number }} COMMENT: ${{ needs.duel.outputs.comment }} STATUS: ${{ needs.duel.outputs.status }} run: | marker="\")" | tail -n 1) + last=$(gh api --paginate "repos/$GITHUB_REPOSITORY/comments" \ + --jq ".[] | select(.user.login == \"github-actions[bot]\") | .body | split(\"\n\")[0] | rtrimstr(\"\r\") | select(startswith(\"$marker status=\"))" | tail -n 1) if [ "$COMMENT" = post ] || { [ -n "$last" ] && [ "$last" != "$marker status=$STATUS -->" ]; }; then { echo "$marker status=$STATUS -->"; cat "$RUNNER_TEMP/report/duel.md"; } > "$RUNNER_TEMP/comment.md" - gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR/comments" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null + gh api --method POST "repos/$GITHUB_REPOSITORY/commits/$GITHUB_SHA/comments" -F body=@"$RUNNER_TEMP/comment.md" > /dev/null fi + + # With c.push_base = :last_passed in .duel.rb, a push is compared with + # the last commit Duel passed, which a branch marks (each branch pushed + # has its own), so slowdowns too small for any one push to fail add up + # until they fail one. This job moves the branch to a push that passed + # (or makes it). It checks nothing out and runs none of the code. To + # accept a regression, move the branch yourself (for main, git push + # origin main:duel/passed/main). + passed: + name: Mark the commit passed + needs: duel + if: ${{ !cancelled() && github.event_name == 'push' && needs.duel.outputs.passed == 'true' }} + runs-on: ubuntu-latest + permissions: + contents: write # to move the branch + steps: + - name: Move the branch to the commit + env: + GH_TOKEN: ${{ github.token }} + BRANCH: duel/passed/${{ github.ref_name }} + run: | + gh api --method PATCH "repos/$GITHUB_REPOSITORY/git/refs/heads/$BRANCH" -f sha="$GITHUB_SHA" -F force=true > /dev/null 2>&1 || + gh api --method POST "repos/$GITHUB_REPOSITORY/git/refs" -f ref="refs/heads/$BRANCH" -f sha="$GITHUB_SHA" > /dev/null diff --git a/Gemfile b/Gemfile index 6f050a8..500c29b 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ gem "rake" group :development do # Duel requires Ruby 3.4 or later - gem "duel", ">= 0.1.1" if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4") + gem "duel", ">= 0.1.2" if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4") gem "stackprof", platforms: :mri end