Skip to content

Commit e6fb3de

Browse files
committed
Implement a make jobserver:
- Fix #9170 - In #9131, we added running `make` in parallel with the `-j` flag. It's problematic because since Bundler installs gems in parallel, we could end up installing `N gem x M processors` which would exhaust the machine resources and causes issues like #9170. In this patch, I have implemented a make [jobserver](https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html), so that each thread installing a gem can take available job slots from the pool. I also want to highlight that with this patch, we can still end up running 2 more jobs than what's available. The reason being that if a gem is waiting for slots to be available, then the whole `bundle install` takes much longer which defeats the whole purpose of the `make -j` optimization. So if there is no more slots available, we compile the extension without parallelization (we still use one cpu though). Example ---------- If `bundle install -j 6` is running, then we have 6 slots available. Each gem with native extension that gets installed may take up to *3* slots (see my reasoning on this number below). In the event where 3 gems with native extensions get installed, 2 gems will consume all 6 slots, leaving the third gem without any. Ultimately, this means that we could use *at maximum 2 more slots than what's available* (e.g. `bundle install -j 3` for 3 gems will use 5 slots). I chose `3` slots per `make` process because after installing multiple gems, I didn't see any benefit to adding more jobs. It's possible that some gems have many `make` recipes that could run more than 3 in parallel, but I don't think this is very frequent.
1 parent ca8ac9f commit e6fb3de

File tree

6 files changed

+190
-8
lines changed

6 files changed

+190
-8
lines changed

bundler/lib/bundler/installer/parallel_installer.rb

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,27 @@ def failed_specs
107107
end
108108

109109
def install_with_worker
110-
enqueue_specs
111-
process_specs until finished_installing?
110+
with_jobserver do
111+
enqueue_specs
112+
process_specs until finished_installing?
113+
end
114+
end
115+
116+
def with_jobserver
117+
r, w = IO.pipe
118+
r.close_on_exec = false
119+
w.close_on_exec = false
120+
w.write("*" * @size)
121+
122+
old_makeflags = ENV["MAKEFLAGS"]
123+
ENV["MAKEFLAGS"] = "#{old_makeflags} --jobserver-auth=#{r.fileno},#{w.fileno}"
124+
125+
yield
126+
ensure
127+
r.close
128+
w.close
129+
130+
old_makeflags ? ENV["MAKEFLAGS"] = old_makeflags : ENV.delete("MAKEFLAGS")
112131
end
113132

114133
def install_serially

bundler/lib/bundler/rubygems_gem_installer.rb

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,18 @@ def generate_bin_script(filename, bindir)
104104
end
105105

106106
def build_jobs
107-
Bundler.settings[:jobs] || super
107+
@jobserver_read_io&.read_nonblock(3, @jobserver_tokens)
108+
available_jobs = @jobserver_tokens.empty? ? nil : @jobserver_tokens.size
109+
110+
available_jobs || Bundler.settings[:jobs] || super
111+
rescue IO::WaitReadable
112+
1
108113
end
109114

110115
def build_extensions
116+
@jobserver_tokens = +""
117+
@jobserver_read_io, @jobserver_write_io = connect_to_jobserver
118+
111119
extension_cache_path = options[:bundler_extension_cache_path]
112120
extension_dir = spec.extension_dir
113121
unless extension_cache_path && extension_dir
@@ -131,6 +139,11 @@ def build_extensions
131139
FileUtils.cp_r extension_dir, extension_cache_path
132140
end
133141
end
142+
ensure
143+
unless @jobserver_tokens.empty?
144+
@jobserver_write_io.write(@jobserver_tokens)
145+
@jobserver_write_io.flush
146+
end
134147
end
135148

136149
def spec
@@ -147,6 +160,15 @@ def gem_checksum
147160

148161
private
149162

163+
def connect_to_jobserver
164+
return unless ENV["MAKEFLAGS"]
165+
read_fd, write_fd = ENV["MAKEFLAGS"].match(/--jobserver-auth=(\d+),(\d+)/)&.captures
166+
167+
return unless read_fd && write_fd
168+
169+
[IO.new(read_fd.to_i, autoclose: false), IO.new(write_fd.to_i, autoclose: false)]
170+
end
171+
150172
def prepare_extension_build(extension_dir)
151173
SharedHelpers.filesystem_access(extension_dir, :create) do
152174
FileUtils.mkdir_p extension_dir
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
# frozen_string_literal: true
2+
3+
require "bundler/installer/parallel_installer"
4+
require "bundler/rubygems_gem_installer"
5+
require "rubygems/remote_fetcher"
6+
require "bundler"
7+
8+
RSpec.describe Bundler::ParallelInstaller do
9+
describe "connect to make jobserver" do
10+
before do
11+
unless Gem::Installer.private_method_defined?(:build_jobs)
12+
skip "This example is runnable when RubyGems::Installer implements `build_jobs`"
13+
end
14+
15+
require "support/artifice/compact_index"
16+
17+
@previous_client = Gem::Request::ConnectionPools.client
18+
Gem::Request::ConnectionPools.client = Gem::Net::HTTP
19+
Gem::RemoteFetcher.fetcher.close_all
20+
21+
build_repo2 do
22+
build_gem "one", &:add_c_extension
23+
build_gem "two", &:add_c_extension
24+
end
25+
26+
gemfile <<~G
27+
source "https://gem.repo2"
28+
29+
gem "one"
30+
gem "two"
31+
G
32+
lockfile <<~L
33+
GEM
34+
remote: https://gem.repo2/
35+
specs:
36+
one (1.0)
37+
two (1.0)
38+
39+
DEPENDENCIES
40+
one
41+
two
42+
L
43+
44+
@old_ui = Bundler.ui
45+
Bundler.ui = Bundler::UI::Silent.new
46+
end
47+
48+
after do
49+
Bundler.ui = @old_ui
50+
Gem::Request::ConnectionPools.client = @previous_client
51+
Artifice.deactivate
52+
end
53+
54+
let(:definition) do
55+
allow(Bundler).to receive(:root) { bundled_app }
56+
57+
definition = Bundler::Definition.build(bundled_app.join("Gemfile"), bundled_app.join("Gemfile.lock"), false)
58+
definition.tap(&:setup_domain!)
59+
end
60+
let(:installer) { Bundler::Installer.new(bundled_app, definition) }
61+
let(:gem_one) { definition.specs.find {|spec| spec.name == "one" } }
62+
let(:gem_two) { definition.specs.find {|spec| spec.name == "two" } }
63+
64+
it "takes all available slots" do
65+
redefine_build_jobs do
66+
Bundler::ParallelInstaller.call(installer, definition.specs, 5, false, true)
67+
end
68+
69+
# Take 3 slots out of the 5 available.
70+
expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3")
71+
# Take the remaining 2 slots.
72+
expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j2")
73+
end
74+
75+
it "fallback to non parallel when no slots are available" do
76+
redefine_build_jobs do
77+
Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true)
78+
end
79+
80+
# Take 3 slots out of the 3 available.
81+
expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3")
82+
# Fallback to one slot (non parallel).
83+
expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j")
84+
end
85+
86+
it "uses one jobs when installing serially" do
87+
Bundler.settings.temporary(jobs: 1) do
88+
Bundler::ParallelInstaller.call(installer, definition.specs, 1, false, true)
89+
end
90+
91+
expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to_not include("make -j")
92+
expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j")
93+
end
94+
95+
it "release the job slots" do
96+
build_repo2 do
97+
build_gem "one", &:add_c_extension
98+
build_gem "two" do |spec|
99+
spec.add_c_extension
100+
spec.add_dependency(:one) # ParallelInstaller will wait for `one` to be fully installed.
101+
end
102+
end
103+
104+
Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true)
105+
106+
# Take 3 slots out of the 3 available.
107+
expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3")
108+
# Take 3 slots that were released.
109+
expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j3")
110+
end
111+
112+
def redefine_build_jobs
113+
old_method = Bundler::RubyGemsGemInstaller.instance_method(:build_jobs)
114+
Bundler::RubyGemsGemInstaller.remove_method(:build_jobs)
115+
116+
gem_one_waiting = true
117+
gem_two_waiting = true
118+
119+
Bundler::RubyGemsGemInstaller.define_method(:build_jobs) do
120+
if spec.name == "one"
121+
value = old_method.bind(self).call
122+
gem_one_waiting = false
123+
sleep(0.1) while gem_two_waiting
124+
elsif spec.name == "two"
125+
sleep(0.1) while gem_one_waiting
126+
value = old_method.bind(self).call
127+
gem_two_waiting = false
128+
end
129+
130+
value
131+
end
132+
133+
yield
134+
ensure
135+
Bundler::RubyGemsGemInstaller.remove_method(:build_jobs)
136+
Bundler::RubyGemsGemInstaller.define_method(:build_jobs, old_method)
137+
end
138+
end
139+
end

bundler/spec/commands/install_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,7 @@ def run
13521352
expect(gem_make_out).not_to include("make -j8")
13531353
end
13541354

1355-
it "pass down the BUNDLE_JOBS to RubyGems when running the compilation of an extension" do
1355+
it "uses 3 slots from the available pool when running the compilation of an extension" do
13561356
ENV.delete("MAKEFLAGS")
13571357

13581358
install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" })
@@ -1362,10 +1362,10 @@ def run
13621362

13631363
gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out"))
13641364

1365-
expect(gem_make_out).to include("make -j8")
1365+
expect(gem_make_out).to include("make -j3")
13661366
end
13671367

1368-
it "uses nprocessors by default" do
1368+
it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set" do
13691369
ENV.delete("MAKEFLAGS")
13701370

13711371
install_gemfile(<<~G)
@@ -1375,7 +1375,7 @@ def run
13751375

13761376
gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out"))
13771377

1378-
expect(gem_make_out).to include("make -j#{Etc.nprocessors + 1}")
1378+
expect(gem_make_out).to include("make -j3")
13791379
end
13801380
end
13811381

bundler/spec/support/windows_tag_group.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ module WindowsTagGroup
9090
"spec/update/gems/fund_spec.rb",
9191
"spec/bundler/stub_specification_spec.rb",
9292
"spec/bundler/retry_spec.rb",
93+
"spec/bundler/installer/parallel_installer_spec.rb",
9394
"spec/bundler/installer/spec_installation_spec.rb",
9495
"spec/bundler/spec_set_spec.rb",
9596
"spec/quality_es_spec.rb",

lib/rubygems/ext/builder.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ def self.make(dest_path, results, make_dir = Dir.pwd, sitedir = nil, targets = [
4141
# nmake doesn't support parallel build
4242
unless is_nmake
4343
have_make_arguments = make_program.size > 1
44+
n_jobs ||= 0
4445

45-
if !have_make_arguments && !ENV["MAKEFLAGS"] && n_jobs
46+
if !have_make_arguments && n_jobs > 1 && !ENV["MAKEFLAGS"]&.match(/-j\d?(\s|\Z)/)
4647
make_program << "-j#{n_jobs}"
4748
end
4849
end

0 commit comments

Comments
 (0)