Skip to content

Commit

Permalink
tap: fix performance regression in *_files_by_name
Browse files Browse the repository at this point in the history
We essentially stopped caching these accidentally and they get
called every time we try to load a cask or formula from the API.
It gets really, really, really slow.

I ran `brew deps --casks --eval-all` before and after the changes.

I let it run for 3 minutes before killing it. No output had been
printed to the screen.

It finished printing all output (pages and pages of it) in less
than a minute.

---

This should match the caching behavior we had before the
recent changes in these two PRs.

- #16777
- #16775
  • Loading branch information
apainintheneck committed Mar 2, 2024
1 parent 0a3549b commit 99d5200
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1211,14 +1211,16 @@ def formula_names
def formula_files_by_name
return super if Homebrew::EnvConfig.no_install_from_api?

tap_path = path.to_s
Homebrew::API::Formula.all_formulae.each_with_object({}) do |item, hash|
name, formula_hash = item
# If there's more than one item with the same path: use the longer one to prioritise more specific results.
existing_path = hash[name]
# Pathname equivalent is slow in a tight loop
new_path = File.join(tap_path, formula_hash.fetch("ruby_source_path"))
hash[name] = Pathname(new_path) if existing_path.nil? || existing_path.to_s.length < new_path.length
@formula_files_by_name ||= begin
tap_path = path.to_s
Homebrew::API::Formula.all_formulae.each_with_object({}) do |item, hash|
name, formula_hash = item
# If there's more than one item with the same path: use the longer one to prioritise more specific results.
existing_path = hash[name]
# Pathname equivalent is slow in a tight loop
new_path = File.join(tap_path, formula_hash.fetch("ruby_source_path"))
hash[name] = Pathname(new_path) if existing_path.nil? || existing_path.to_s.length < new_path.length
end
end
end

Expand Down Expand Up @@ -1279,7 +1281,7 @@ def cask_tokens
def cask_files_by_name
return super if Homebrew::EnvConfig.no_install_from_api?

Homebrew::API::Cask.all_casks.each_with_object({}) do |item, hash|
@cask_files_by_name ||= Homebrew::API::Cask.all_casks.each_with_object({}) do |item, hash|

Check warning on line 1284 in Library/Homebrew/tap.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/tap.rb#L1284

Added line #L1284 was not covered by tests
name, cask_hash = item
# If there's more than one item with the same path: use the longer one to prioritise more specific results.
existing_path = hash[name]
Expand Down

0 comments on commit 99d5200

Please sign in to comment.