Skip to content

Fix crashing benchmarks and fail CI on crashes - #239

Merged
JuanVqz merged 4 commits into
mainfrom
feature/clean-benchmark-runs
Sep 23, 2026
Merged

JuanVqz merged 4 commits into
mainfrom
feature/clean-benchmark-runs

Conversation

@JuanVqz

@JuanVqz JuanVqz commented Sep 23, 2026

Copy link
Copy Markdown
Member

Summary

  • CI was green even when benchmarks crashed: the Rakefile ignored each benchmark's exit status. The Fix CI by running benchmarks in Docker #238 run (35819054595) had 7 files crashing on at least one Ruby.
  • This fixes those crashes, makes the run fail when a benchmark crashes, and makes PR checks run only what a PR changes.

Changes

Step 1: ostruct on Ruby 4.0+

  • Both OpenStruct benchmarks crashed on 4.0, ruby-head, jruby-head and truffleruby-head with cannot load such file -- ostruct (LoadError): it is no longer a default gem, and bundle exec only loads gems in the Gemfile.
  • gem 'ostruct' if RUBY_VERSION >= '3.4'. The if matters: without it Ruby 2.1 installs ostruct 0.1.0, which does not parse there, and older Rubies would benchmark the gem instead of the ostruct they ship.

Step 2: benchmarks that need a newer Ruby

  • dig-vs-[]-vs-fetch.rb (2.1, 2.2): only the Hash#dig report is skipped, the other five still run.
  • slice-native-vs-before-native.rb (2.1 to 2.4, JRuby 9.1): only the native Hash#slice report is skipped, so the pre-native versions the file is about still run there.
  • remove-extra-spaces-or-other-chars.rb (2.1, 2.2): <<~ made the file fail to parse. A plain heredoc builds the same string, checked byte for byte.
  • bsearch-vs-find.rb (JRuby): the default Java heap cap is too small for its 100M element array (JRuby 9.1 failed in CI at 3554MB, JRuby 10 locally at 2232MB). Both JRubies get JRUBY_OPTS=-J-Xmx6g.
  • Sharing on JRuby 9.1: on main (SHARE=1), every JRuby 9.1 benchmark crashed after printing its results with OpenSSL::SSL::SSLError: Received fatal alert: handshake_failure: its bundled jruby-openssl cannot connect to ips.fastruby.io. With Step 3 that would turn every main run red, since PR runs never share. JRuby before 9.2 now gets a newer jruby-openssl (0.14.6 resolves); JRuby 10 already connects and keeps its own.

Step 3: fail the run when a benchmark crashes

  • The Rakefile runs every file, then exits non-zero listing the ones that failed, so one crash does not hide the others.
  • docker/run-benchmarks.sh with file arguments did stop at the first failing file; it now reports the same way.

Step 4: PRs run only what they change

  • paths filter: only changes to code/**/*.rb, Gemfile, Rakefile, compose.yaml, docker/** or the workflow run Benchmarks. Of the 15 merged PRs before Fix CI by running benchmarks in Docker #238, 6 changed only README/CONTRIBUTING.
  • On a PR, only the changed benchmark files run on every Ruby (about 1 to 2 min per job instead of about 22). A change to a shared file runs everything, a PR that only deletes benchmarks runs nothing, and pushes to main always run everything.
  • File names reach the container through an env var with globbing off, so names like dig-vs-[]-vs-fetch.rb pass through as is and cannot inject shell.

Test plan

  • Locally (arm64 Mac): each crash reproduced before its fix and passing after, on the Rubies listed above.
  • Full suite on all 18 Rubies with benchmark-ips cut to 0.1s + 0.2s (a crash check, not real numbers): 17 exit 0. jruby_head failed only bsearch-vs-find.rb (heap), which passes with the heap change.
  • A deliberately broken benchmark makes both the full run and file mode exit 1 and name the file, on Ruby 3.2 and 2.1.
  • The file selection was tested on real merge commits: one benchmark + README, Gemfile, docker/, [] in names, README only. actionlint is clean.
  • The share fix: a GET to ips.fastruby.io from JRuby 9.1 fails the handshake without the gem and returns 200 with it; jruby-head, 2.1, 3.2 and 4.0 don't get the gem and still connect. No reports were posted while testing.
  • This PR changes the Gemfile, Rakefile and workflow, so its own CI runs the full suite on all 18 Rubies with Step 3 in place: all green means nothing crashes.
  • Not verifiable before merge: that a later benchmark-only PR runs just its files.

On Ruby 4.0, ruby-head, jruby-head and truffleruby-head, both OpenStruct
benchmarks crashed with "cannot load such file -- ostruct (LoadError)":
ostruct is no longer a default gem, and bundle exec only loads gems that
are in the Gemfile.

The gem is conditional because an unconditional one breaks Ruby 2.1
(ostruct 0.1.0 uses &.) and would replace the ostruct older Rubies ship.
Crashes seen in CI, now fixed:

- dig-vs-[]-vs-fetch.rb on 2.1 and 2.2: skip only the Hash#dig report
  (Ruby 2.3+), the other five still run.
- slice-native-vs-before-native.rb on 2.1 to 2.4 and JRuby 9.1: skip only
  the native Hash#slice report (Ruby 2.5+), so the pre-native versions the
  file is about still run there.
- remove-extra-spaces-or-other-chars.rb on 2.1 and 2.2: <<~ (Ruby 2.3+)
  made the file fail to parse. A plain heredoc builds the same string
  (checked byte for byte).
- bsearch-vs-find.rb on JRuby: the default Java heap cap (1/4 of memory)
  is too small for its 100M element array. It failed on JRuby 9.1 in CI
  (3554MB cap) and on JRuby 10 locally (2232MB). Both now get -J-Xmx6g.
- On main (SHARE=1), JRuby 9.1 crashed every benchmark when sharing:
  its bundled jruby-openssl fails the TLS handshake with ips.fastruby.io
  ("Received fatal alert: handshake_failure"). The Gemfile gives JRuby
  before 9.2 a newer jruby-openssl (0.14.6 resolves). JRuby 10 already
  connects, so it keeps its own.
The Rakefile ignored each system(...) result, so CI stayed green with 7
benchmarks crashing across Rubies. It now runs every file, then exits
non-zero listing the ones that failed, so one crash does not hide others.

docker/run-benchmarks.sh with file arguments stopped at the first failing
file (set -e). It now runs them all and reports the same way.
@JuanVqz
JuanVqz force-pushed the feature/clean-benchmark-runs branch from 70a7f74 to 25bc9d3 Compare September 23, 2026 19:13
Every PR ran all 68 benchmarks on 18 Rubies (about 22 min per job, 18 of
the org's 20 concurrent jobs), even for README-only changes.

- paths filter: only changes to benchmarks, the Gemfile, Rakefile,
  compose.yaml, docker/ or this workflow run it, on PRs and on main.
- On a PR, only the changed benchmark files run, unless a shared file
  changed, then everything runs. A PR that only deletes benchmarks runs
  nothing. Pushes to main always run everything.
- File names reach the container through an env var with globbing off,
  so names like dig-vs-[]-vs-fetch.rb pass through as is and a PR cannot
  inject shell through a file name.
@JuanVqz
JuanVqz force-pushed the feature/clean-benchmark-runs branch from 25bc9d3 to 8468a2f Compare September 23, 2026 19:17
@JuanVqz
JuanVqz marked this pull request as ready for review September 23, 2026 19:47
@JuanVqz
JuanVqz merged commit 1350517 into main Sep 23, 2026
18 checks passed
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.

1 participant