From 32dcea236c5589c13e59000619dbfafff7347557 Mon Sep 17 00:00:00 2001 From: Erik Berlin Date: Sun, 25 Oct 2026 08:01:36 -0700 Subject: [PATCH 1/2] Add Duel performance regression guards Add Duel benchmarks in duel/ that guard the hot paths: URI parsing, normalization and joining, building and merging headers, building and writing requests, multipart encoding, parsing responses with llhttp, chaining and merging options, and whole requests end to end. The end-to-end benchmarks serve fixed responses from memory through the socket_class option, so they time steadily. One request over a loopback TCP connection is judged on its allocation counts alone. Add the Perf workflow, which compares each pull request with main and comments with the report, the duel Rake task, the Duel badge, and include .duel.rb in RuboCop. Duel needs Ruby 3.4 or later, so the Gemfile and Rakefile skip it on older Rubies. 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 | 92 +++++++++++++++++++++ .rubocop.yml | 5 ++ .rubocop/style.yml | 1 + Gemfile | 4 + README.md | 1 + Rakefile | 7 ++ duel/client_duel.rb | 90 ++++++++++++++++++++ duel/duel_helper.rb | 165 +++++++++++++++++++++++++++++++++++++ duel/headers_duel.rb | 41 +++++++++ duel/options_duel.rb | 18 ++++ duel/request_duel.rb | 54 ++++++++++++ duel/response_duel.rb | 54 ++++++++++++ duel/uri_duel.rb | 31 +++++++ 14 files changed, 580 insertions(+) create mode 100644 .duel.rb create mode 100644 .github/workflows/perf.yml create mode 100644 duel/client_duel.rb create mode 100644 duel/duel_helper.rb create mode 100644 duel/headers_duel.rb create mode 100644 duel/options_duel.rb create mode 100644 duel/request_duel.rb create mode 100644 duel/response_duel.rb create mode 100644 duel/uri_duel.rb diff --git a/.duel.rb b/.duel.rb new file mode 100644 index 00000000..55a14e0f --- /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" + + # 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 00000000..1a03329c --- /dev/null +++ b/.github/workflows/perf.yml @@ -0,0 +1,92 @@ +name: Perf + +on: + pull_request: + push: + branches: [main] + 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 + +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 0dc6d0b4..157067eb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,6 +13,11 @@ AllCops: DisplayStyleGuide: true NewCops: enable TargetRubyVersion: 3.2 + Include: + - '.duel.rb' +inherit_mode: + merge: + - Include # Feature has no initialize, so super() is a no-op in subclasses Lint/MissingSuper: diff --git a/.rubocop/style.yml b/.rubocop/style.yml index c1d223aa..bdbac531 100644 --- a/.rubocop/style.yml +++ b/.rubocop/style.yml @@ -12,6 +12,7 @@ Style/FormatStringToken: Style/OneClassPerFile: Exclude: + - 'duel/**/*.rb' - 'test/**/*.rb' Style/RescueModifier: diff --git a/Gemfile b/Gemfile index 973a7f3f..4ddb24cc 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,10 @@ group :development do gem "debug", platform: :mri gem "nokogiri", require: false + + # 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 diff --git a/README.md b/README.md index 3218b83c..fcc8f19b 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ [![Mutant][mutant-image]][mutant-link] [![Test][test-image]][test-link] [![Typecheck][typecheck-image]][typecheck-link] +[![Duel](https://raw.githubusercontent.com/sferik/duel-ruby/main/assets/badge.svg)](https://github.com/sferik/duel-ruby) [Documentation] diff --git a/Rakefile b/Rakefile index b9dc253f..fc78646a 100644 --- a/Rakefile +++ b/Rakefile @@ -102,3 +102,10 @@ namespace :mutant do end task default: %i[test mutant rubocop verify_measurements typecheck] + +begin + require "duel/rake_task" + Duel::RakeTask.new +rescue LoadError + # Duel requires Ruby 3.4 or later +end diff --git a/duel/client_duel.rb b/duel/client_duel.rb new file mode 100644 index 00000000..64a33974 --- /dev/null +++ b/duel/client_duel.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Whole requests, end to end: the session and client, building and +# writing the request, parsing the response, any features, and reading +# the body. A MemoryServer (see duel_helper.rb) serves fixed responses +# from memory, so nothing waits on a socket. + +duel "client: GET a JSON response" do + once do + server = MemoryServer.new(http_response(json_document(10), headers: api_response_headers)) + @http = HTTP::Session.new(socket_class: server) + @http.get(api_url).to_s + end + @http.get(api_url).to_s +end + +duel "client: POST JSON and parse the reply" do + once do + server = MemoryServer.new(http_response(json_document(10), status: "201 Created", headers: api_response_headers)) + @http = HTTP::Session.new(socket_class: server) + @body = { name: "Jane", email: "jane@example.com", tags: %w[a b c] } + @http.post(api_url, json: @body).parse + end + @http.post(api_url, json: @body).parse +end + +# Streamed, as a large body is read: reading it whole grows one string +# by reallocating it, whose cost varies with each process's heap. +duel "client: stream a 128 KB body" do + once do + server = MemoryServer.new(http_response(text_of(1 << 17), headers: { "Content-Type" => "text/plain" })) + @http = HTTP::Session.new(socket_class: server) + @http.get(api_url).body.each { |_chunk| nil } + end + @http.get(api_url).body.each { |_chunk| nil } +end + +duel "client: GET a chunked body" do + once do + bytes = http_response(json_document(100), headers: api_response_headers, chunks: 1024) + @http = HTTP::Session.new(socket_class: MemoryServer.new(bytes)) + @http.get(api_url).to_s + end + @http.get(api_url).to_s +end + +duel "client: GET on a persistent connection" do + once do + server = MemoryServer.new(http_response(json_document(10), headers: api_response_headers)) + # A long keep-alive, so the one connection is reused by every request. + @http = HTTP::Session.new(socket_class: server).persistent("http://api.example.com", timeout: 86_400) + @http.get("/v1/users/42").to_s + end + @http.get("/v1/users/42").to_s +end + +duel "client: GET a gzipped body with auto_inflate" do + once do + headers = api_response_headers.merge("Content-Encoding" => "gzip") + server = MemoryServer.new(http_response(gzip(json_document(500)), headers: headers)) + @http = HTTP::Session.new(socket_class: server).use(:auto_inflate) + @http.get(api_url).to_s + end + @http.get(api_url).to_s +end + +duel "client: follow a redirect" do + once do + server = MemoryServer.new( + http_response("", status: "302 Found", headers: { "Location" => "/v2/users/42" }), + http_response(json_document(10), headers: api_response_headers) + ) + @http = HTTP::Session.new(socket_class: server).follow + @http.get(api_url).to_s + end + @http.get(api_url).to_s +end + +loopback = lambda do + @server = LoopbackServer.new(http_response(json_document(10), headers: api_response_headers)) + @http = HTTP::Session.new(socket_class: @server) +end + +# Through the kernel, so judged on its counts alone. +duel "client: GET over a loopback TCP connection", + only: :count, setup: loopback, before_sample: -> { @server.close_peers }, teardown: -> { @server.close } do + @http.get(api_url).to_s +end diff --git a/duel/duel_helper.rb b/duel/duel_helper.rb new file mode 100644 index 00000000..e1794bb5 --- /dev/null +++ b/duel/duel_helper.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +require "socket" +require "stringio" +require "zlib" + +# A stand-in for a server, for the socket_class option: each connection +# that the client opens serves the next of a fixed list of responses from +# memory. No system calls, so the client's whole round trip (building and +# writing the request, parsing the response, and reading its body) runs +# in Ruby and times steadily. +class MemoryServer + def initialize(*responses) + @responses = responses.map { |response| response.b.freeze } + @next = 0 + end + + def open(_host, _port) + response = @responses.fetch(@next) + @next = (@next + 1) % @responses.size + MemorySocket.new(response) + end +end + +# One connection to a MemoryServer. It discards what the client writes. +# Each request that it's sent starts the same response again, so a +# persistent connection gets a fresh one for every request. +class MemorySocket + def initialize(response) + @response = response + @offset = response.bytesize + @closed = false + end + + def write(data) + @offset = 0 + data.bytesize + end + + def readpartial(size, buffer = nil) + raise EOFError if @offset >= @response.bytesize + + chunk = @response.byteslice(@offset, size) + @offset += chunk.bytesize + buffer ? buffer.replace(chunk) : chunk + end + + def close + @closed = true + end + + def closed? + @closed + end +end + +# Where a request is written to: it counts the bytes and keeps none. +class NullSink + attr_reader :bytes + + def initialize + @bytes = 0 + end + + def write(data) + @bytes += data.bytesize + data.bytesize + end + alias << write +end + +# The bytes of an HTTP/1.1 response with these headers and body. With +# chunks, the body is sent chunked, in chunks of that many bytes. +def http_response(body = "", status: "200 OK", headers: {}, chunks: nil) + head = "HTTP/1.1 #{status}\r\n" + headers.each { |name, value| head << "#{name}: #{value}\r\n" } + if chunks + head << "Transfer-Encoding: chunked\r\n\r\n" + slices(body.b, chunks).each { |chunk| head << "#{chunk.bytesize.to_s(16)}\r\n#{chunk}\r\n" } + head << "0\r\n\r\n" + else + head << "Content-Length: #{body.bytesize}\r\n\r\n" << body.b + end + head.b +end + +# The string, in slices of up to this many bytes. +def slices(string, size) + (0...string.bytesize).step(size).map { |offset| string.byteslice(offset, size) } +end + +# The headers of a typical response from an API. +def api_response_headers + { + "Date" => "Sat, 24 Oct 2026 12:00:00 GMT", + "Content-Type" => "application/json; charset=utf-8", + "Cache-Control" => "max-age=0, private, must-revalidate", + "ETag" => 'W/"5d41402abc4b2a76b9719d911017c592"', + "Vary" => "Accept, Accept-Encoding", + "X-Request-Id" => "0f8fad5b-d9cb-469f-a165-70867728950e", + "X-Runtime" => "0.012345", + "Strict-Transport-Security" => "max-age=31536000; includeSubDomains" + } +end + +# A JSON document of this many records, about 100 bytes each. +def json_document(records) + items = Array.new(records) do |i| + %({"id":#{i},"name":"user#{i}","email":"user#{i}@example.com","active":true,"score":#{(i * 7) % 100}}) + end + "[#{items.join(',')}]" +end + +# Text of this many bytes, the same every time. +def text_of(bytes) + line = "The quick brown fox jumps over the lazy dog. 0123456789\n" + (line * ((bytes / line.bytesize) + 1)).byteslice(0, bytes) +end + +def gzip(string) + io = StringIO.new(+"") + gz = Zlib::GzipWriter.new(io) + gz.mtime = 0 + gz.write(string) + gz.close + io.string +end + +# The URL that requests go to. Plain http, as a MemoryServer speaks no +# TLS. +def api_url + "http://api.example.com/v1/users/42?include=posts&page=2" +end + +# A real TCP server on the loopback interface, served from the +# benchmark's own thread: each connection that the client opens is +# accepted at once and sent a fixed response, before the client writes +# its request. Only for benchmarks judged on counts, as system calls time +# too unsteadily. +class LoopbackServer + def initialize(response) + @response = response.b.freeze + @server = TCPServer.new("127.0.0.1", 0) + @port = @server.addr[1] + @peers = [] + end + + def open(_host, _port) + socket = TCPSocket.open("127.0.0.1", @port) + peer = @server.accept + peer.write(@response) + @peers << peer + socket + end + + # Closes the server's ends of the connections so far. + def close_peers + @peers.each(&:close).clear + end + + def close + close_peers + @server.close + end +end diff --git a/duel/headers_duel.rb b/duel/headers_duel.rb new file mode 100644 index 00000000..08b1f862 --- /dev/null +++ b/duel/headers_duel.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Every request builds its headers, and every response's headers are +# looked up by name. Names are normalized through a per-thread cache, +# which each benchmark warms in `once`. + +duel "headers: build 5 from a hash" do + once do + @hash = { "Accept" => "application/json", "Authorization" => "Bearer abc123", + "User-Agent" => "MyApp/1.0", :content_type => "text/plain", "X-Request-Id" => "42" } + HTTP::Headers.coerce(@hash) + end + HTTP::Headers.coerce(@hash) +end + +duel "headers: build 50 from a hash" do + once do + @hash = Array.new(50) { |i| ["X-Custom-Header-#{i}", "value-#{i}"] }.to_h + HTTP::Headers.coerce(@hash) + end + HTTP::Headers.coerce(@hash) +end + +duel "headers: look up a response's headers" do + once { @headers = HTTP::Headers.coerce(api_response_headers) } + @headers["Content-Type"] + @headers["ETag"] + @headers.get("Vary") + @headers.include?("Transfer-Encoding") +end + +duel "headers: merge request headers" do + once do + @defaults = HTTP::Headers.coerce("Accept" => "application/json", "User-Agent" => "MyApp/1.0") + @extra = { "Authorization" => "Bearer abc123", "Accept" => "text/html", "X-Request-Id" => "42" } + @defaults.merge(@extra) + end + @defaults.merge(@extra) +end diff --git a/duel/options_duel.rb b/duel/options_duel.rb new file mode 100644 index 00000000..3588dada --- /dev/null +++ b/duel/options_duel.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Chaining options, as in HTTP.headers(...).auth(...).get(...), builds a +# new set of options and a new session at each step. + +duel "options: chain headers, auth, accept, timeout" do + HTTP.headers("X-Request-Id" => "42").auth("Bearer abc123").accept(:json).timeout(5) +end + +duel "options: merge a request's options" do + once do + @defaults = HTTP::Options.new(headers: { "Accept" => "application/json" }, follow: true) + @request = { headers: { "Authorization" => "Bearer abc123" }, params: { page: 2 } } + end + @defaults.merge(@request) +end diff --git a/duel/request_duel.rb b/duel/request_duel.rb new file mode 100644 index 00000000..dade1771 --- /dev/null +++ b/duel/request_duel.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Building a request from options (its URL, headers and body), and +# writing it out, happen once per request. The writes go to a sink in +# memory, not a socket. + +duel "request: build a GET" do + once { @options = HTTP::Options.new } + HTTP::Request::Builder.new(@options).build(:get, api_url) +end + +duel "request: build a GET with query params" do + once { @options = HTTP::Options.new(params: { q: "ruby http", page: 2, per_page: 50 }) } + HTTP::Request::Builder.new(@options).build(:get, api_url) +end + +duel "request: build a POST with a JSON body" do + once { @options = HTTP::Options.new(json: { name: "Jane", email: "jane@example.com", tags: %w[a b c], age: 42 }) } + HTTP::Request::Builder.new(@options).build(:post, api_url) +end + +duel "request: build a POST with a form" do + once { @options = HTTP::Options.new(form: { name: "Jane", email: "jane@example.com", tags: %w[a b c], age: 42 }) } + HTTP::Request::Builder.new(@options).build(:post, api_url) +end + +duel "request: write a GET" do + once { @request = HTTP::Request::Builder.new(HTTP::Options.new).build(:get, api_url) } + @request.stream(NullSink.new) +end + +duel "request: write a POST with an 8 KB body" do + once { @request = HTTP::Request.new(verb: :post, uri: api_url, body: text_of(8192)) } + @request.stream(NullSink.new) +end + +duel "request: write a chunked body of 16 KB" do + once do + @request = HTTP::Request.new(verb: :post, uri: api_url, headers: { "Transfer-Encoding" => "chunked" }, + body: Array.new(16) { text_of(1024) }) + end + @request.stream(NullSink.new) +end + +duel "request: encode a multipart upload of 16 KB" do + once do + @file = text_of(16_384) + @boundary = ("-" * 21) + ("0" * 42) # fixed, where Multipart would pick a random one + end + part = HTTP::FormData::Part.new(@file, filename: "notes.txt", content_type: "text/plain") + HTTP::FormData::Multipart.new({ title: "Notes", file: part }, boundary: @boundary).to_s +end diff --git a/duel/response_duel.rb b/duel/response_duel.rb new file mode 100644 index 00000000..ec74cbe4 --- /dev/null +++ b/duel/response_duel.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Parsing a response from fixed bytes with llhttp, the way a connection +# feeds it: one read of up to Connection::BUFFER_SIZE (16 KB), then the +# body taken out. A connection keeps its parser and resets it for each +# response, as these do. (Large bodies are guarded by "client: stream a +# 128 KB body": parsing one alone is mostly copying memory, which times +# too unsteadily to judge.) + +duel "response: parse a small response" do + once do + @parser = HTTP::Response::Parser.new + @bytes = http_response("OK", headers: { "Content-Type" => "text/plain" }) + end + @parser.reset + @parser << @bytes + @parser.read(HTTP::Connection::BUFFER_SIZE) +end + +duel "response: parse 30 headers and a JSON body" do + once do + @parser = HTTP::Response::Parser.new + headers = api_response_headers.merge(Array.new(22) { |i| ["X-Header-#{i}", "value #{i}"] }.to_h) + @bytes = http_response(json_document(10), headers: headers) + end + @parser.reset + @parser << @bytes + @parser.read(HTTP::Connection::BUFFER_SIZE) +end + +duel "response: parse a chunked body of 16 KB" do + once do + @parser = HTTP::Response::Parser.new + @bytes = http_response(text_of(16_384), headers: { "Content-Type" => "text/plain" }, chunks: 1024) + end + @parser.reset + @parser << @bytes + @parser.read(HTTP::Connection::BUFFER_SIZE) +end + +duel "response: build one and read its metadata" do + once do + @request = HTTP::Request.new(verb: :get, uri: "https://api.example.com/v1/users/42") + @headers = api_response_headers.merge("Content-Length" => "512") + end + response = HTTP::Response.new(status: 200, version: "1.1", headers: @headers, body: "", request: @request) + response.mime_type + response.charset + response.content_length + response.chunked? + response.status.success? +end diff --git a/duel/uri_duel.rb b/duel/uri_duel.rb new file mode 100644 index 00000000..cea1b65f --- /dev/null +++ b/duel/uri_duel.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require_relative "duel_helper" + +# Every request parses and normalizes its URL, and every redirect joins +# the Location header onto the URL that it came from. + +duel "uri: parse a typical URL" do + once { @url = "https://api.example.com/v1/users/42?include=posts&page=2" } + HTTP::URI.parse(@url) +end + +duel "uri: normalize a typical URL" do + once { @url = "https://api.example.com/v1/users/42?include=posts&page=2" } + HTTP::URI::NORMALIZER.call(@url) +end + +duel "uri: normalize case, port and dot segments" do + once { @url = "HTTP://Api.Example.COM:80/v1/users/../teams/./42/../../orgs/7?x=1" } + HTTP::URI::NORMALIZER.call(@url) +end + +duel "uri: normalize a URL with a 2 KB query" do + once { @url = "https://api.example.com/search?#{Array.new(100) { |i| "key#{i}=value#{i}abc" }.join('&')}" } + HTTP::URI::NORMALIZER.call(@url) +end + +duel "uri: join a relative redirect" do + once { @base = HTTP::URI.parse("https://api.example.com/v1/users/42?include=posts") } + @base.join("../orgs/7?page=3") +end From b2d53f0808af6e3e6976193442d19f6661a59f4f 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 55a14e0f..81f25fe2 100644 --- a/.duel.rb +++ b/.duel.rb @@ -11,7 +11,6 @@ # this says otherwise). # c.library = "http" - # 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 1a03329c..9ae9cc37 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -17,6 +17,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: @@ -29,9 +34,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 @@ -55,15 +62,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 @@ -80,13 +89,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 4ddb24cc..eba575d3 100644 --- a/Gemfile +++ b/Gemfile @@ -11,7 +11,7 @@ group :development do gem "nokogiri", require: false # 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