Don't load tmpdir and fileutils at require time - #282
Merged
Conversation
`require "tmpdir"` was never used -- nothing in the gem calls Dir.mktmpdir
or Dir.tmpdir -- but it transitively pulls in fileutils and etc on every
require of mixlib-shellout.
`fileutils` is used in exactly one place, set_cgroup, which only runs when
the :cgroup option is set and cgroup v2 is mounted. Load it in the parent
inside fork_subprocess instead. It deliberately goes there rather than in
set_cgroup itself: set_cgroup runs post-fork, and taking the require lock
after forking a threaded parent can deadlock.
Both of these are paid by every consumer of the gem -- chef-infra, ohai,
test-kitchen, inspec -- on every process start.
$ ruby -e 'require "mixlib/shellout"' # 30 runs, ruby 4.0.6
before median 12.96 ms +2148 heap objects, +7 files
after median 5.39 ms +0 heap objects, +5 files
58% faster, and it no longer allocates.
Signed-off-by: Tim Smith <tsmith84@proton.me>
The spellcheck job only scans files a PR touches, so any change to lib/mixlib/shellout/unix.rb or lib/mixlib/shellout.rb fails on identifiers that have been in those files for years: cgroupv, ducktype, endgrent, getgrent, LOGNAME, pgid, secondarygroups, seconderies, sgids and WNOHANG. "proccess" was flagged too, but that one is an actual typo in a comment rather than a word worth teaching the dictionary, so fix it instead. Signed-off-by: Tim Smith <tsmith84@proton.me>
tas50
force-pushed
the
perf/lazy-require-fileutils
branch
from
August 28, 2026 03:57
db5fa9b to
423050e
Compare
This was referenced Aug 28, 2026
tpowell-progress
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
lib/mixlib/shellout.rbhas requiredtmpdirsince forever, but nothing in the gem ever callsDir.mktmpdirorDir.tmpdir:It's not free —
tmpdirtransitively pulls infileutilsandetc, which is where essentially all of the require-time allocation comes from.fileutilsitself is used, but in exactly one place:set_cgroup, which only runs when the:cgroupoption is set and cgroup v2 is mounted. This moves that require tofork_subprocess, gated oncgroupbeing set.Worth calling out why it goes in
fork_subprocessand not inset_cgroupitself:set_cgroupruns inside the forked child.requiretakes a global lock, and if the parent forked while another thread held that lock, the child would block on it forever. Loading in the parent keeps it lazy without the fork-safety hazard.This matters beyond this gem — it's paid by every consumer (chef-infra, ohai, test-kitchen, inspec) on every process start.
Benchmark
30 cold processes each, ruby 4.0.6, arm64-darwin:
58% faster, and it no longer allocates at require time.
Verification
bundle exec rspec— 147 examples, 0 failures (unchanged frommain)bundle exec cookstyle --chefstyle -c .rubocop.yml— no offensesrun_commandworks without them::cgrouppath still loadsFileUtils, and that it loads in the parent before the fork.spec_helper.rbalready requirestmpdirexplicitly, so the specs never relied on the library leaking it.