tap: optimise CoreTap#formula_files_by_name

[Bo98]

In a loop that's iterated nearly 7000 times, Pathname#/ is actually quite slow. It does a lot more than one might think it does: https://github.com/ruby/ruby/blob/24fe22a5da21c9df8584a4ce6b6d1ce18ac41cc2/ext/pathname/lib/pathname.rb#L362-L402 (+ regex matching in chop_basename). We however don't need logic like .. resolving.

We can turn 0.2s to 0.02s by using File.join and later making it Pathname. There's still a bit of string copying going on, meaning it can spike to 0.1s if it triggers the GC, but it's nevertheless much better than before.

[apainintheneck]

It produces the same results which is good.

brew(main):001:1* class CoreTap < AbstractCoreTap
brew(main):002:2*   def formula_files_by_name_2
brew(main):003:2*     return super if Homebrew::EnvConfig.no_install_from_api?
brew(main):004:2*
brew(main):005:2*     tap_path = path.to_s
brew(main):006:3*     Homebrew::API::Formula.all_formulae.each_with_object({}) do |item, hash|
brew(main):007:3*       name, formula_hash = item
brew(main):008:3*       # If there's more than one item with the same path: use the longer one to prioritise more specif
ic results.
brew(main):009:3*       existing_path = hash[name]
brew(main):010:3*       new_path = File.join(tap_path, formula_hash["ruby_source_path"]) # Pathname equivalent is slow i
n a tight loop
brew(main):011:3*       hash[name] = Pathname(new_path) if existing_path.nil? || existing_path.to_s.length < new_path.le
ngth
brew(main):012:2*     end
brew(main):013:1*   end
brew(main):014:0> end
=> :formula_files_by_name_2
brew(main):015:0>
brew(main):016:0> ENV["HOMEBREW_NO_AUTO_UPDATE"] = "1"
=> "1"
brew(main):017:0> CoreTap.instance.formula_files_by_name == CoreTap.instance.formula_files_by_name_2
=> true

And it's much faster.

# frozen_string_literal: true

ENV["HOMEBREW_NO_AUTO_UPDATE"] = "1"

class CoreTap < AbstractCoreTap
  def formula_files_by_name_2
    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]
      new_path = File.join(tap_path, formula_hash["ruby_source_path"]) # Pathname equivalent is slow in a tight loop
      hash[name] = Pathname(new_path) if existing_path.nil? || existing_path.to_s.length < new_path.length
    end
  end
end

# Pre-load these to avoid it messing with timing.
Homebrew::API::Formula.all_formulae
CoreTap.instance

require "benchmark"

results = Benchmark.bmbm do |x|
  x.report("formula_files_by_name") do
    CoreTap.instance.formula_files_by_name
  end

  x.report("formula_files_by_name_2") do
    CoreTap.instance.formula_files_by_name_2
  end
end

puts results

$ brew ruby /Users/kevinrobell/Desktop/formula_files_by_name_benchmark.rb
Rehearsal -----------------------------------------------------------
formula_files_by_name     0.179240   0.016622   0.195862 (  0.195965)
formula_files_by_name_2   0.017780   0.002905   0.020685 (  0.020690)
-------------------------------------------------- total: 0.216547sec

                              user     system      total        real
formula_files_by_name     0.102959   0.000239   0.103198 (  0.103244)
formula_files_by_name_2   0.018503   0.000053   0.018556 (  0.018587)
  0.102959   0.000239   0.103198 (  0.103244)
  0.018503   0.000053   0.018556 (  0.018587)

[apainintheneck]

Great work here as always.

[MikeMcQuaid]

Great work @Bo98 and thanks for review @apainintheneck!