Skip to content

Stop rebuilding the defaults Hash on every option read - #438

Merged
tpowell-progress merged 1 commit into
chef:mainfrom
tas50:perf/options-accessor
Aug 31, 2026
Merged

Stop rebuilding the defaults Hash on every option read#438
tpowell-progress merged 1 commit into
chef:mainfrom
tas50:perf/options-accessor

Conversation

@tas50

@tas50 tas50 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Every one of the 13 generated option readers ran:

options[option] || options[option.to_s] || default_options[option]

So each read that was not explicitly set allocated a String from to_s and then built a fresh 5-entry Hash — including an ENV lookup — just to pull one key back out of it. Options#for_ps1? reads two options, so it did that twice per call, and for_ps1? is on the hot path for both script generation and artifact filtering.

This freezes the String key once when the reader is defined, and looks up a single default instead of materializing the whole Hash.

license_id still reads ENV["CHEF_LICENSE_KEY"] on every access, so environment changes are picked up exactly as before.

The || chain is deliberately kept rather than switched to nil? checks, so an explicitly false option still falls through to its default exactly as it does today.

Measurements

Ruby 4.0.6:

before after speedup allocations
#shell_type (default) 445 ns 180 ns 2.5x 3.02 → 0.02
#product_version (default) 450 ns 140 ns 3.2x 3 → 0
#for_ps1? 907 ns 327 ns 2.8x 7 → 1
#latest_version? 483 ns 174 ns 2.8x
.new 4094 ns 2632 ns 1.6x 30 → 16
#validate_options! 1193 ns 867 ns 1.4x

#product_name and other explicitly-set reads are unchanged at ~78 ns, as expected — they never reached the defaults branch.

Verification

  • 576 unit examples and the functional spec pass; chefstyle clean.
  • Explicit assertions that defaults, explicitly-set values, String-keyed options, nil options and the CHEF_LICENSE_KEY environment fallback all resolve identically, including picking up an environment change between two Options.new calls.
  • All 176 golden SHA256 digests match main.

@tas50
tas50 requested review from a team and jaymzh as code owners August 28, 2026 03:56
Every one of the 13 generated option readers ran:

  options[option] || options[option.to_s] || default_options[option]

so each read that was not explicitly set allocated a String from `to_s` and
then built a fresh 5 entry Hash (including an ENV lookup) just to pull one key
out of it. `Options#for_ps1?` reads two options, so it did that twice.

Freeze the String key once when the reader is defined, and look up a single
default instead of materializing the whole Hash. `license_id` still reads
ENV["CHEF_LICENSE_KEY"] on every access, so changes to the environment are
picked up exactly as before.

The `||` chain is kept rather than switched to nil checks so that an
explicitly `false` option still falls through to its default, as it does today.

Measured on Ruby 4.0.6:

  #shell_type       445 ns -> 180 ns   2.5x   3.02 -> 0.02 allocations
  #product_version  450 ns -> 140 ns   3.2x   3    -> 0    allocations
  #for_ps1?         907 ns -> 327 ns   2.8x   7    -> 1    allocation
  #latest_version?  483 ns -> 174 ns   2.8x
  .new             4094 ns -> 2632 ns  1.6x   30   -> 16   allocations
  validate_options! 1193 ns -> 867 ns  1.4x

Output is unchanged: 576 unit examples pass and all 176 golden SHA256 digests
match main.

Signed-off-by: Tim Smith <tsmith84@proton.me>
@tas50
tas50 force-pushed the perf/options-accessor branch from 18dc925 to bc8c7d2 Compare August 28, 2026 03:58
@tpowell-progress
tpowell-progress merged commit 299cb64 into chef:main Aug 31, 2026
74 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.

2 participants