From 99d5200db3ad314ce631601500199d240742eb8d Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Fri, 1 Mar 2024 19:26:39 -0800 Subject: [PATCH] tap: fix performance regression in *_files_by_name 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. - https://github.com/Homebrew/brew/pull/16777 - https://github.com/Homebrew/brew/pull/16775 --- Library/Homebrew/tap.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index 399719b5d1..fed9a01d94 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -1211,14 +1211,16 @@ class CoreTap < AbstractCoreTap 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 @@ -1279,7 +1281,7 @@ class CoreCaskTap < AbstractCoreTap 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| 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]