Skip to content

Lazily load mixlib-shellout - #436

Open
tas50 wants to merge 1 commit into
chef:mainfrom
tas50:perf/lazy-shellout
Open

Lazily load mixlib-shellout#436
tas50 wants to merge 1 commit into
chef:mainfrom
tas50:perf/lazy-shellout

Conversation

@tas50

@tas50 tas50 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

mixlib-shellout costs ~9 ms to require and pulls in etc, fileutils, tmpdir and fcntl — 7 files. It is used in exactly one place: shelling out from Mixlib::Install.detect_platform. This loads it there instead of making every consumer pay for it at require time.

Measurements

Ruby 4.0.6, minimum of 15 fresh subprocesses:

require time loaded features
before 54.79 ms 240
after 45.40 ms 233

Compatibility note — please read

This is the one deferral in this series with an externally observable effect, so it is deliberately split into its own PR.

Code that calls Mixlib::ShellOut after only requiring mixlib-install, without requiring mixlib-shellout itself, will now get a NameError until it adds the require. That is relying on a transitive require, but it is easy to do by accident — our own functional spec did exactly that, and is fixed in this PR.

The three main consumers (chef, chef-cli, test-kitchen) all require mixlib-shellout directly, so I believe the risk is low. But it is a real behaviour change for ~9 ms, and it is entirely reasonable to close this one and keep the other load-time PRs in this series. Happy either way.

Verification

  • 576 unit examples and the functional spec pass; chefstyle clean.
  • Mixlib::Install.detect_platform verified working in a bare process: Mixlib::ShellOut is undefined at load, defined after the call, and the returned platform hash is correct.
  • All 176 golden SHA256 digests match main.

@tas50
tas50 requested review from a team and jaymzh as code owners August 28, 2026 03:55
mixlib-shellout costs ~9ms to require and pulls in etc, fileutils, tmpdir and
fcntl (7 files). It is used in exactly one place -- shelling out from
`Mixlib::Install.detect_platform` -- so load it there rather than making every
consumer pay for it at require time.

Measured on Ruby 4.0.6 (min of 15 fresh subprocesses):

  before   54.79 ms   240 loaded features
  after    45.40 ms   233 loaded features

Compatibility note: this is the one deferral in this series with an observable
effect. Code that called `Mixlib::ShellOut` after only requiring
mixlib-install, without requiring mixlib-shellout itself, will now get a
NameError until it adds the require. Our own functional spec did exactly that,
and is fixed here. Please drop this commit if that risk is not worth ~9ms.

Generated output is unchanged: all 176 golden SHA256 digests match main.

Signed-off-by: Tim Smith <tsmith84@proton.me>
@tas50
tas50 force-pushed the perf/lazy-shellout branch from f594d47 to 675b7d8 Compare August 28, 2026 03:58
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