Skip to content

perf/upgrade: avoid linear scans of core formula names (~20% faster)#22977

Merged
dduugg merged 1 commit into
mainfrom
upgrade-formula-lookup-perf
Jul 6, 2026
Merged

perf/upgrade: avoid linear scans of core formula names (~20% faster)#22977
dduugg merged 1 commit into
mainfrom
upgrade-formula-lookup-perf

Conversation

@dduugg

@dduugg dduugg commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

install and upgrade are the 2nd and 3rd most-run brew commands in the last 365 days of analytics (5.20% and 4.94% of all invocations). Profiling brew upgrade --dry-run with stackprof showed two hot spots in the formula resolution path they share:

  1. Formulary::FromAPILoader.try_new called Homebrew::API.formula_names.exclude?(name) for every formula lookup. Each call builds a fresh array of all ~7,500 core formula names via Hash#keys and then scans it linearly. Together, Array#include? (9.6%) and Hash#keys (3.3%) accounted for ~13% of total wall time.
  2. Formula#linked_keg checked the found path with .present?. Pathname#present? delegates to Pathname#empty?, which performs a Dir.empty? syscall on the directory, ~3% of total wall time in aggregate.

Changes

  • Add Homebrew::API.formula_name?(name), an O(1) Hash#key? membership test, and use it in Formulary::FromAPILoader.try_new and Formulary.tap_formula_name_type instead of scanning formula_names.
  • Return the found linked_keg path via a truthiness check instead of .present?, avoiding a directory-read syscall per formula.

brew upgrade --dry-run output is byte-identical before and after. The same lookup path also serves install, outdated, info and any other command resolving core formulae by name.

Benchmarks (Hyperfine, 10 runs each, warmup 2, HOMEBREW_SORBET_RUNTIME unset, 9 taps and ~30 outdated formulae):

brew upgrade --dry-run Mean Range
Before 4.697 s ± 0.127 s 4.507-4.955 s
After 3.738 s ± 0.045 s 3.658-3.810 s

~20% decrease in wall time (~1 s saved per run).

Remaining call sites and a cop to ban the pattern

A comprehensive scan for other linear membership checks against the API name lists found more, now also converted (with Homebrew::API.cask_token? added to mirror formula_name?):

  • bundle/installer.rb: formula_names.exclude?/cask_tokens.exclude? per unqualified Brewfile entry.
  • cli/named_args.rb: formula_names.include?/cask_tokens.include? per named argument (reached when the core taps are not installed, the default setup).
  • utils/analytics.rb: one check per brew info formula and one per cask.
  • dev-cmd/edit.rb: four membership checks on named arguments.
  • uninstall.rb: repeated include? against a local formula_names array while scanning etc leftovers; now a Set.
  • cask/cask_loader.rb: Cask::CaskLoader::FromAPILoader.try_new (the cask counterpart of the Formulary hot spot) and tap_cask_token_type.

As suggested in review, this now includes an internal autocorrecting Homebrew/ApiNameMembership cop that bans include?/exclude? on Homebrew::API.formula_names/Homebrew::API.cask_tokens. The cop earned its keep immediately: the cask_loader.rb, analytics.rb cask and one of the edit.rb call sites above were caught by the cop after the manual scan missed them.

Each converted check drops from ~48 μs (plus a fresh ~7,500-element Hash#keys array allocation) to ~5 μs. Unlike the Formulary site these are per-argument or per-entry rather than per-formula-resolution, so command-level benchmarks are unchanged (brew upgrade --dry-run re-measured at 3.92 s vs 3.91 s before this sweep, within noise); the savings scale with Brewfile size, named-argument count and installed-cask count. tap_auditor.rb and cask/audit.rb scan Tap#formula_names (a different, tap-local list) and are left alone.


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Claude Code was used to profile brew upgrade --dry-run with stackprof, identify the hot spots, implement the changes and write the new spec. Verified by comparing before/after brew upgrade --dry-run output for byte-identical results, clean before/after Hyperfine benchmarks on otherwise identical trees, and running brew lgtm locally.


@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! I'd suggest an internal Rubocop for this to "ban" the existing pattern.

@dduugg dduugg marked this pull request as ready for review July 6, 2026 14:54
Copilot AI review requested due to automatic review settings July 6, 2026 14:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR speeds up common formula/cask resolution paths used by brew install/upgrade by avoiding repeated linear scans over API name lists and eliminating an unnecessary filesystem emptiness check.

Changes:

  • Adds Homebrew::API.formula_name? and Homebrew::API.cask_token? for O(1) membership checks (Hash#key?) and updates call sites to use them.
  • Updates Formula#linked_keg to rely on truthiness instead of Pathname#present? (avoids a Dir.empty? syscall).
  • Adds an internal autocorrecting RuboCop cop (Homebrew/ApiNameMembership) plus tests and Sorbet RBI to prevent regressions.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Library/Homebrew/api.rb Adds formula_name?/cask_token? predicate APIs backed by Hash#key?.
Library/Homebrew/formulary.rb Replaces formula_names include/exclude scans with formula_name?.
Library/Homebrew/cask/cask_loader.rb Replaces cask_tokens include/exclude scans with cask_token?.
Library/Homebrew/bundle/installer.rb Switches Brewfile “available without tap?” checks to predicates.
Library/Homebrew/cli/named_args.rb Uses predicates when core taps aren’t installed and API install is allowed.
Library/Homebrew/dev-cmd/edit.rb Uses predicates to decide whether to auto-tap core/core-cask when editing.
Library/Homebrew/utils/analytics.rb Uses predicates to guard analytics API lookups for formulae/casks.
Library/Homebrew/uninstall.rb Converts excluded-name list to a Set for faster membership checks.
Library/Homebrew/formula.rb Avoids Pathname#present? emptiness check in linked_keg.
Library/Homebrew/rubocops/api_name_membership.rb New cop forbidding include?/exclude? scans on API name lists; autocorrects to predicates.
Library/Homebrew/rubocops/all.rb Requires the new RuboCop cop.
Library/Homebrew/test/api_spec.rb Adds unit coverage for the new predicate methods.
Library/Homebrew/test/rubocops/api_name_membership_spec.rb Adds cop offense + autocorrect + negative coverage tests.
Library/Homebrew/test/dev-cmd/edit_spec.rb Updates stubbing to match predicate-based implementation.
Library/Homebrew/test/bundle/installer_spec.rb Updates stubbing to match predicate-based implementation.
Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/api_name_membership.rbi Adds generated RBI for the new cop’s node matcher method.
Files not reviewed (1)
  • Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/api_name_membership.rbi: File type not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dduugg dduugg force-pushed the upgrade-formula-lookup-perf branch from 0074c25 to 1e81b7f Compare July 6, 2026 16:47
Comment thread Library/Homebrew/formula.rb
@dduugg

dduugg commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Great idea! I'd suggest an internal Rubocop for this to "ban" the existing pattern.

Good call, it's narrower in the patterns that it can find than what this PR fixes, but I've added one to the PR.

@dduugg dduugg enabled auto-merge July 6, 2026 16:56
@dduugg dduugg added this pull request to the merge queue Jul 6, 2026
Merged via the queue into main with commit fa2c6df Jul 6, 2026
41 checks passed
@dduugg dduugg deleted the upgrade-formula-lookup-perf branch July 6, 2026 17:27
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.

3 participants