Skip to content

Commit

Permalink
[rubygems/rubygems] Reduce allocations when installing gems with bundler
Browse files Browse the repository at this point in the history
```
==> memprof.after.txt <==
Total allocated: 1.13 MB (2352 objects)
Total retained:  10.08 kB (78 objects)

==> memprof.before.txt <==
Total allocated: 46.27 MB (38439 objects)
Total retained:  9.94 kB (75 objects)
```

Yes, we were allocating 45MB of arrays in `dependencies_installed?`,
it was accidentally cubic.

rubygems/rubygems@13ab874388
  • Loading branch information
segiddins authored and matzbot committed Nov 26, 2023
1 parent cc5d1bf commit 08308fe
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
13 changes: 9 additions & 4 deletions lib/bundler/installer/parallel_installer.rb
Expand Up @@ -42,8 +42,7 @@ def ignorable_dependency?(dep)

# Checks installed dependencies against spec's dependencies to make
# sure needed dependencies have been installed.
def dependencies_installed?(all_specs)
installed_specs = all_specs.select(&:installed?).map(&:name)
def dependencies_installed?(installed_specs)
dependencies.all? {|d| installed_specs.include? d.name }
end

Expand Down Expand Up @@ -183,8 +182,14 @@ def require_tree_for_spec(spec)
# previously installed specifications. We continue until all specs
# are installed.
def enqueue_specs
@specs.select(&:ready_to_enqueue?).each do |spec|
if spec.dependencies_installed? @specs
installed_specs = {}
@specs.each do |spec|
next unless spec.installed?
installed_specs[spec.name] = true
end

@specs.each do |spec|
if spec.ready_to_enqueue? && spec.dependencies_installed?(installed_specs)
spec.state = :enqueued
worker_pool.enq spec
end
Expand Down
6 changes: 4 additions & 2 deletions spec/bundler/bundler/installer/spec_installation_spec.rb
Expand Up @@ -47,7 +47,8 @@ def a_spec.full_name
all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :installed? => false, :all_dependencies => [], :type => :production)]
spec = described_class.new(dep)
allow(spec).to receive(:all_dependencies).and_return(dependencies)
expect(spec.dependencies_installed?(all_specs)).to be_truthy
installed_specs = all_specs.select(&:installed?).map {|s| [s.name, true] }.to_h
expect(spec.dependencies_installed?(installed_specs)).to be_truthy
end
end

Expand All @@ -59,7 +60,8 @@ def a_spec.full_name
all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :installed? => false, :all_dependencies => [], :type => :production)]
spec = described_class.new(dep)
allow(spec).to receive(:all_dependencies).and_return(dependencies)
expect(spec.dependencies_installed?(all_specs)).to be_falsey
installed_specs = all_specs.select(&:installed?).map {|s| [s.name, true] }.to_h
expect(spec.dependencies_installed?(installed_specs)).to be_falsey
end
end
end
Expand Down

0 comments on commit 08308fe

Please sign in to comment.