From e6fb3deebb940d82ed7f0246673ebe262a3acc5c Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Thu, 18 Dec 2025 14:43:05 +0100 Subject: [PATCH] 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. --- .../bundler/installer/parallel_installer.rb | 23 ++- bundler/lib/bundler/rubygems_gem_installer.rb | 24 ++- .../installer/parallel_installer_spec.rb | 139 ++++++++++++++++++ bundler/spec/commands/install_spec.rb | 8 +- bundler/spec/support/windows_tag_group.rb | 1 + lib/rubygems/ext/builder.rb | 3 +- 6 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 bundler/spec/bundler/installer/parallel_installer_spec.rb diff --git a/bundler/lib/bundler/installer/parallel_installer.rb b/bundler/lib/bundler/installer/parallel_installer.rb index d10e5ec92403..c2b9900e2884 100644 --- a/bundler/lib/bundler/installer/parallel_installer.rb +++ b/bundler/lib/bundler/installer/parallel_installer.rb @@ -107,8 +107,27 @@ def failed_specs end def install_with_worker - enqueue_specs - process_specs until finished_installing? + with_jobserver do + enqueue_specs + process_specs until finished_installing? + end + end + + def with_jobserver + r, w = IO.pipe + r.close_on_exec = false + w.close_on_exec = false + w.write("*" * @size) + + old_makeflags = ENV["MAKEFLAGS"] + ENV["MAKEFLAGS"] = "#{old_makeflags} --jobserver-auth=#{r.fileno},#{w.fileno}" + + yield + ensure + r.close + w.close + + old_makeflags ? ENV["MAKEFLAGS"] = old_makeflags : ENV.delete("MAKEFLAGS") end def install_serially diff --git a/bundler/lib/bundler/rubygems_gem_installer.rb b/bundler/lib/bundler/rubygems_gem_installer.rb index 64ce6193d3d1..392a96321c56 100644 --- a/bundler/lib/bundler/rubygems_gem_installer.rb +++ b/bundler/lib/bundler/rubygems_gem_installer.rb @@ -104,10 +104,18 @@ def generate_bin_script(filename, bindir) end def build_jobs - Bundler.settings[:jobs] || super + @jobserver_read_io&.read_nonblock(3, @jobserver_tokens) + available_jobs = @jobserver_tokens.empty? ? nil : @jobserver_tokens.size + + available_jobs || Bundler.settings[:jobs] || super + rescue IO::WaitReadable + 1 end def build_extensions + @jobserver_tokens = +"" + @jobserver_read_io, @jobserver_write_io = connect_to_jobserver + extension_cache_path = options[:bundler_extension_cache_path] extension_dir = spec.extension_dir unless extension_cache_path && extension_dir @@ -131,6 +139,11 @@ def build_extensions FileUtils.cp_r extension_dir, extension_cache_path end end + ensure + unless @jobserver_tokens.empty? + @jobserver_write_io.write(@jobserver_tokens) + @jobserver_write_io.flush + end end def spec @@ -147,6 +160,15 @@ def gem_checksum private + def connect_to_jobserver + return unless ENV["MAKEFLAGS"] + read_fd, write_fd = ENV["MAKEFLAGS"].match(/--jobserver-auth=(\d+),(\d+)/)&.captures + + return unless read_fd && write_fd + + [IO.new(read_fd.to_i, autoclose: false), IO.new(write_fd.to_i, autoclose: false)] + end + def prepare_extension_build(extension_dir) SharedHelpers.filesystem_access(extension_dir, :create) do FileUtils.mkdir_p extension_dir diff --git a/bundler/spec/bundler/installer/parallel_installer_spec.rb b/bundler/spec/bundler/installer/parallel_installer_spec.rb new file mode 100644 index 000000000000..04544c49dbd4 --- /dev/null +++ b/bundler/spec/bundler/installer/parallel_installer_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require "bundler/installer/parallel_installer" +require "bundler/rubygems_gem_installer" +require "rubygems/remote_fetcher" +require "bundler" + +RSpec.describe Bundler::ParallelInstaller do + describe "connect to make jobserver" do + before do + unless Gem::Installer.private_method_defined?(:build_jobs) + skip "This example is runnable when RubyGems::Installer implements `build_jobs`" + end + + require "support/artifice/compact_index" + + @previous_client = Gem::Request::ConnectionPools.client + Gem::Request::ConnectionPools.client = Gem::Net::HTTP + Gem::RemoteFetcher.fetcher.close_all + + build_repo2 do + build_gem "one", &:add_c_extension + build_gem "two", &:add_c_extension + end + + gemfile <<~G + source "https://gem.repo2" + + gem "one" + gem "two" + G + lockfile <<~L + GEM + remote: https://gem.repo2/ + specs: + one (1.0) + two (1.0) + + DEPENDENCIES + one + two + L + + @old_ui = Bundler.ui + Bundler.ui = Bundler::UI::Silent.new + end + + after do + Bundler.ui = @old_ui + Gem::Request::ConnectionPools.client = @previous_client + Artifice.deactivate + end + + let(:definition) do + allow(Bundler).to receive(:root) { bundled_app } + + definition = Bundler::Definition.build(bundled_app.join("Gemfile"), bundled_app.join("Gemfile.lock"), false) + definition.tap(&:setup_domain!) + end + let(:installer) { Bundler::Installer.new(bundled_app, definition) } + let(:gem_one) { definition.specs.find {|spec| spec.name == "one" } } + let(:gem_two) { definition.specs.find {|spec| spec.name == "two" } } + + it "takes all available slots" do + redefine_build_jobs do + Bundler::ParallelInstaller.call(installer, definition.specs, 5, false, true) + end + + # Take 3 slots out of the 5 available. + expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") + # Take the remaining 2 slots. + expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j2") + end + + it "fallback to non parallel when no slots are available" do + redefine_build_jobs do + Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true) + end + + # Take 3 slots out of the 3 available. + expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") + # Fallback to one slot (non parallel). + expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j") + end + + it "uses one jobs when installing serially" do + Bundler.settings.temporary(jobs: 1) do + Bundler::ParallelInstaller.call(installer, definition.specs, 1, false, true) + end + + expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to_not include("make -j") + expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j") + end + + it "release the job slots" do + build_repo2 do + build_gem "one", &:add_c_extension + build_gem "two" do |spec| + spec.add_c_extension + spec.add_dependency(:one) # ParallelInstaller will wait for `one` to be fully installed. + end + end + + Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true) + + # Take 3 slots out of the 3 available. + expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") + # Take 3 slots that were released. + expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j3") + end + + def redefine_build_jobs + old_method = Bundler::RubyGemsGemInstaller.instance_method(:build_jobs) + Bundler::RubyGemsGemInstaller.remove_method(:build_jobs) + + gem_one_waiting = true + gem_two_waiting = true + + Bundler::RubyGemsGemInstaller.define_method(:build_jobs) do + if spec.name == "one" + value = old_method.bind(self).call + gem_one_waiting = false + sleep(0.1) while gem_two_waiting + elsif spec.name == "two" + sleep(0.1) while gem_one_waiting + value = old_method.bind(self).call + gem_two_waiting = false + end + + value + end + + yield + ensure + Bundler::RubyGemsGemInstaller.remove_method(:build_jobs) + Bundler::RubyGemsGemInstaller.define_method(:build_jobs, old_method) + end + end +end diff --git a/bundler/spec/commands/install_spec.rb b/bundler/spec/commands/install_spec.rb index ae651bf981c7..007e21f5c3a7 100644 --- a/bundler/spec/commands/install_spec.rb +++ b/bundler/spec/commands/install_spec.rb @@ -1352,7 +1352,7 @@ def run expect(gem_make_out).not_to include("make -j8") end - it "pass down the BUNDLE_JOBS to RubyGems when running the compilation of an extension" do + it "uses 3 slots from the available pool when running the compilation of an extension" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }) @@ -1362,10 +1362,10 @@ def run gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out")) - expect(gem_make_out).to include("make -j8") + expect(gem_make_out).to include("make -j3") end - it "uses nprocessors by default" do + it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G) @@ -1375,7 +1375,7 @@ def run gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out")) - expect(gem_make_out).to include("make -j#{Etc.nprocessors + 1}") + expect(gem_make_out).to include("make -j3") end end diff --git a/bundler/spec/support/windows_tag_group.rb b/bundler/spec/support/windows_tag_group.rb index 8eb0a749dafb..3a36e6943229 100644 --- a/bundler/spec/support/windows_tag_group.rb +++ b/bundler/spec/support/windows_tag_group.rb @@ -90,6 +90,7 @@ module WindowsTagGroup "spec/update/gems/fund_spec.rb", "spec/bundler/stub_specification_spec.rb", "spec/bundler/retry_spec.rb", + "spec/bundler/installer/parallel_installer_spec.rb", "spec/bundler/installer/spec_installation_spec.rb", "spec/bundler/spec_set_spec.rb", "spec/quality_es_spec.rb", diff --git a/lib/rubygems/ext/builder.rb b/lib/rubygems/ext/builder.rb index 350daf1e16d7..5ffe9d2dc00b 100644 --- a/lib/rubygems/ext/builder.rb +++ b/lib/rubygems/ext/builder.rb @@ -41,8 +41,9 @@ def self.make(dest_path, results, make_dir = Dir.pwd, sitedir = nil, targets = [ # nmake doesn't support parallel build unless is_nmake have_make_arguments = make_program.size > 1 + n_jobs ||= 0 - if !have_make_arguments && !ENV["MAKEFLAGS"] && n_jobs + if !have_make_arguments && n_jobs > 1 && !ENV["MAKEFLAGS"]&.match(/-j\d?(\s|\Z)/) make_program << "-j#{n_jobs}" end end