From e3a47bfca1757a0f264947f9ccd4146cb103e7ca Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Sun, 25 Oct 2026 10:25:29 -0700 Subject: [PATCH 1/2] 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/2] 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