Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement caching for dependency expansion #10887

Merged
merged 1 commit into from Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions Library/Homebrew/dependencies.rbi
@@ -0,0 +1,9 @@
# typed: strict

class Dependencies < SimpleDelegator
include Kernel
end

class Requirements < SimpleDelegator
include Kernel
end
14 changes: 5 additions & 9 deletions Library/Homebrew/dependencies_helpers.rb
Expand Up @@ -35,15 +35,11 @@ def args_includes_ignores(args)
end

def recursive_includes(klass, root_dependent, includes, ignores)
type = if klass == Dependency
:dependencies
elsif klass == Requirement
:requirements
else
raise ArgumentError, "Invalid class argument: #{klass}"
end
raise ArgumentError, "Invalid class argument: #{klass}" if klass != Dependency && klass != Requirement

cache_key = "recursive_includes_#{includes}_#{ignores}"

root_dependent.send("recursive_#{type}") do |dependent, dep|
klass.expand(root_dependent, cache_key: cache_key) do |dependent, dep|
if dep.recommended?
klass.prune if ignores.include?("recommended?") || dependent.build.without?(dep)
elsif dep.optional?
Expand All @@ -57,7 +53,7 @@ def recursive_includes(klass, root_dependent, includes, ignores)

# If a tap isn't installed, we can't find the dependencies of one of
# its formulae, and an exception will be thrown if we try.
if type == :dependencies &&
if klass == Dependency &&
dep.is_a?(TapDependency) &&
!dep.tap.installed?
Dependency.keep_but_prune_recursive_deps
Expand Down
18 changes: 13 additions & 5 deletions Library/Homebrew/dependency.rb
Expand Up @@ -11,6 +11,7 @@ class Dependency

extend Forwardable
include Dependable
extend Cachable

attr_reader :name, :tags, :env_proc, :option_names

Expand Down Expand Up @@ -87,12 +88,17 @@ class << self
# `[dependent, dep]` pairs to allow callers to apply arbitrary filters to
# the list.
# The default filter, which is applied when a block is not given, omits
# optionals and recommendeds based on what the dependent has asked for.
def expand(dependent, deps = dependent.deps, &block)
# optionals and recommendeds based on what the dependent has asked for
def expand(dependent, deps = dependent.deps, cache_key: nil, &block)
# Keep track dependencies to avoid infinite cyclic dependency recursion.
@expand_stack ||= []
@expand_stack.push dependent.name

if cache_key.present?
cache[cache_key] ||= {}
return cache[cache_key][dependent.full_name].dup if cache[cache_key][dependent.full_name]
end

expanded_deps = []

deps.each do |dep|
Expand All @@ -104,18 +110,20 @@ def expand(dependent, deps = dependent.deps, &block)
when :skip
next if @expand_stack.include? dep.name

expanded_deps.concat(expand(dep.to_formula, &block))
expanded_deps.concat(expand(dep.to_formula, cache_key: cache_key, &block))
when :keep_but_prune_recursive_deps
expanded_deps << dep
else
next if @expand_stack.include? dep.name

expanded_deps.concat(expand(dep.to_formula, &block))
expanded_deps.concat(expand(dep.to_formula, cache_key: cache_key, &block))
expanded_deps << dep
end
end

merge_repeats(expanded_deps)
expanded_deps = merge_repeats(expanded_deps)
cache[cache_key][dependent.full_name] = expanded_deps.dup if cache_key.present?
expanded_deps
ensure
@expand_stack.pop
end
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/dev-cmd/bottle.rb
Expand Up @@ -310,6 +310,8 @@ def bottle_formula(f, args:)
Formula.clear_cache
Keg.clear_cache
Tab.clear_cache
Dependency.clear_cache
Requirement.clear_cache
tab = Tab.for_keg(keg)
original_tab = tab.dup
tab.poured_from_bottle = false
Expand Down
26 changes: 23 additions & 3 deletions Library/Homebrew/formula.rb
Expand Up @@ -165,7 +165,7 @@ class Formula
# during the installation of a {Formula}. This is annoying but the result of
# state that we're trying to eliminate.
# @return [BuildOptions]
attr_accessor :build
attr_reader :build

# Whether this formula should be considered outdated
# if the target of the alias it was installed with has since changed.
Expand Down Expand Up @@ -222,10 +222,28 @@ def active_spec=(spec_sym)
spec = send(spec_sym)
raise FormulaSpecificationError, "#{spec_sym} spec is not available for #{full_name}" unless spec

old_spec_sym = @active_spec_sym
@active_spec = spec
@active_spec_sym = spec_sym
validate_attributes!
@build = active_spec.build

return if spec_sym == old_spec_sym

Dependency.clear_cache
Requirement.clear_cache
end

# @private
def build=(build_options)
old_options = @build
@build = build_options

return if old_options.used_options == build_options.used_options &&
old_options.unused_options == build_options.unused_options

Dependency.clear_cache
Requirement.clear_cache
end

private
Expand Down Expand Up @@ -1657,13 +1675,15 @@ def print_tap_action(options = {})
# means if a depends on b then b will be ordered before a in this list
# @private
def recursive_dependencies(&block)
Dependency.expand(self, &block)
cache_key = "Formula#recursive_dependencies" unless block
Dependency.expand(self, cache_key: cache_key, &block)
end

# The full set of Requirements for this formula's dependency tree.
# @private
def recursive_requirements(&block)
Requirement.expand(self, &block)
cache_key = "Formula#recursive_requirements" unless block
Requirement.expand(self, cache_key: cache_key, &block)
end

# Returns a Keg for the opt_prefix or installed_prefix if they exist.
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/formula_installer.rb
Expand Up @@ -572,8 +572,8 @@ def expand_requirements
unsatisfied_reqs = Hash.new { |h, k| h[k] = [] }
req_deps = []
formulae = [formula]
formula_deps_map = Dependency.expand(formula)
.index_by(&:name)
formula_deps_map = formula.recursive_dependencies
.index_by(&:name)

while (f = formulae.pop)
runtime_requirements = runtime_requirements(f)
Expand Down
6 changes: 6 additions & 0 deletions Library/Homebrew/options.rb
Expand Up @@ -114,6 +114,12 @@ def *(other)
@options.to_a * other
end

def ==(other)
instance_of?(other.class) &&
to_a == other.to_a
end
alias eql? ==

def empty?
@options.empty?
end
Expand Down
9 changes: 8 additions & 1 deletion Library/Homebrew/requirement.rb
Expand Up @@ -15,6 +15,7 @@ class Requirement
extend T::Sig

include Dependable
extend Cachable

attr_reader :tags, :name, :cask, :download

Expand Down Expand Up @@ -219,7 +220,12 @@ class << self
# the list.
# The default filter, which is applied when a block is not given, omits
# optionals and recommendeds based on what the dependent has asked for.
def expand(dependent, &block)
def expand(dependent, cache_key: nil, &block)
if cache_key.present?
cache[cache_key] ||= {}
return cache[cache_key][dependent.full_name].dup if cache[cache_key][dependent.full_name]
end

reqs = Requirements.new

formulae = dependent.recursive_dependencies.map(&:to_formula)
Expand All @@ -233,6 +239,7 @@ def expand(dependent, &block)
end
end

cache[cache_key][dependent.full_name] = reqs.dup if cache_key.present?
reqs
end

Expand Down
12 changes: 7 additions & 5 deletions Library/Homebrew/test/formula_spec.rb
Expand Up @@ -166,8 +166,7 @@
end

build_values_with_no_installed_alias = [
nil,
BuildOptions.new({}, {}),
BuildOptions.new(Options.new, f.options),
Tab.new(source: { "path" => f.path.to_s }),
]
build_values_with_no_installed_alias.each do |build|
Expand Down Expand Up @@ -201,7 +200,10 @@
url "foo-1.0"
end

build_values_with_no_installed_alias = [nil, BuildOptions.new({}, {}), Tab.new(source: { "path" => f.path })]
build_values_with_no_installed_alias = [
BuildOptions.new(Options.new, f.options),
Tab.new(source: { "path" => f.path }),
]
build_values_with_no_installed_alias.each do |build|
f.build = build
expect(f.installed_alias_path).to be nil
Expand Down Expand Up @@ -405,7 +407,7 @@
f = formula alias_path: alias_path do
url "foo-1.0"
end
f.build = BuildOptions.new({}, {})
f.build = BuildOptions.new(Options.new, f.options)

expect(f.alias_path).to eq(alias_path)
expect(f.installed_alias_path).to be nil
Expand Down Expand Up @@ -802,7 +804,7 @@

expect(Set.new(f1.recursive_requirements)).to eq(Set[])

f1.build = BuildOptions.new(["--with-xcode"], f1.options)
f1.build = BuildOptions.new(Options.create(["--with-xcode"]), f1.options)

expect(Set.new(f1.recursive_requirements)).to eq(Set[xcode])

Expand Down
4 changes: 4 additions & 0 deletions Library/Homebrew/test/spec_helper.rb
Expand Up @@ -185,6 +185,8 @@ def find_files
Formula.clear_cache
Keg.clear_cache
Tab.clear_cache
Dependency.clear_cache
Requirement.clear_cache
FormulaInstaller.clear_attempted
FormulaInstaller.clear_installed

Expand Down Expand Up @@ -229,6 +231,8 @@ def find_files
Formula.clear_cache
Keg.clear_cache
Tab.clear_cache
Dependency.clear_cache
Requirement.clear_cache

FileUtils.rm_rf [
*TEST_DIRECTORIES,
Expand Down