Skip to content

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Aug 20, 2025

This fixes LoadError and warnings coming from loading fiddle on benchmarks with Gemfile.

While #368 was enough to fix LoadError for benchmarks without Gemfile, it wasn't for ones with Gemfile.

Before

Ruby 3.4

/home/k0kubun/src/github.com/Shopify/yjit-bench/harness/harness-common.rb:77: warning: fiddle was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
You can add fiddle to your Gemfile or gemspec to silence this warning.
MAXRSS: 105.8MiB

Ruby 3.5

Failed to get max RSS: fiddle is not part of the bundle. Add it to your Gemfile.

After

Ruby 3.4

MAXRSS: 105.9MiB

Ruby 3.5

MAXRSS: 118.3MiB

Why force_activate

At this point, it doesn't seem like a good idea to rely on fiddle in the default harness, which is no longer a standard library. However, a procfs alternative (VmHWM in) /proc/$pid/status is not only inaccurate but also unavailable for macOS. So there's a reason to keep relying on ru_maxrss of getrusage, which seems impossible to call using only stdlib.

While it's possible to just add fiddle to every single benchmark with Gemfile, I don't want to spend effort making sure every new benchmark adds fiddle to the Gemfile.

So I want to go with this PR until it stops working. When fiddle is removed from bundled gems, we'd have to add it to every Gemfile though.

@k0kubun k0kubun force-pushed the force-fiddle branch 4 times, most recently from 1c148fb to 1afa976 Compare August 21, 2025 00:05
@k0kubun k0kubun marked this pull request as ready for review August 21, 2025 00:08
@k0kubun k0kubun requested a review from a team August 21, 2025 00:14
@tenderworks tenderworks merged commit de8d4c7 into ruby:main Aug 21, 2025
4 checks passed
@k0kubun k0kubun deleted the force-fiddle branch August 21, 2025 17:51
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.

2 participants