diff --git a/benchmarks/gvl_release_acquire/benchmark.rb b/benchmarks/gvl_release_acquire/benchmark.rb index 3c368477..bf8ad256 100644 --- a/benchmarks/gvl_release_acquire/benchmark.rb +++ b/benchmarks/gvl_release_acquire/benchmark.rb @@ -1,7 +1,7 @@ require_relative "../../harness/loader" run_benchmark(5) do |num_rs, ractor_args| - output = File.open("/dev/null", "wb") + output = File.open(File::NULL, "wb") input = File.open("/dev/zero", "rb") 100_000.times do output.write(input.read(10)) diff --git a/benchmarks/knucleotide/benchmark.rb b/benchmarks/knucleotide/benchmark.rb index cf6234b4..94bbb028 100644 --- a/benchmarks/knucleotide/benchmark.rb +++ b/benchmarks/knucleotide/benchmark.rb @@ -37,6 +37,13 @@ def find_seq(seq, s) class Worker def initialize(&block) + # Platforms without fork (Windows) compute the result sequentially in this process, + # so their timings are not comparable with the parallel ones. + unless Process.respond_to?(:fork) + @result = yield + return + end + @r, @w = IO.pipe @p = Process.fork do @r.close @@ -47,6 +54,8 @@ def initialize(&block) end def result + return @result unless @p + ret = @r.read @r.close Process.wait(@p) diff --git a/benchmarks/railsbench/benchmark.rb b/benchmarks/railsbench/benchmark.rb index 89561d48..f89f91a3 100644 --- a/benchmarks/railsbench/benchmark.rb +++ b/benchmarks/railsbench/benchmark.rb @@ -8,7 +8,7 @@ # and this app's db/seeds.rb will delete and repopulate # the database, so rows shouldn't accumulate. Dir.chdir __dir__ -use_gemfile extra_setup_cmd: "bin/rails db:migrate db:seed" +use_gemfile extra_setup_cmd: "ruby bin/rails db:migrate db:seed" # Windows cannot run bin/rails by its shebang require_relative 'config/environment' diff --git a/harness/harness-common.rb b/harness/harness-common.rb index 8125f2e7..161140d6 100644 --- a/harness/harness-common.rb +++ b/harness/harness-common.rb @@ -1,10 +1,11 @@ require 'rbconfig' +require 'fileutils' require_relative '../misc/stats' # Ensure the ruby in PATH is the ruby running this, so we can safely shell out to other commands ruby_in_path = `ruby -e 'print RbConfig.ruby'` unless ruby_in_path == RbConfig.ruby - ENV["PATH"] = "#{File.dirname(RbConfig.ruby)}:#{ENV["PATH"]}" + ENV["PATH"] = "#{File.dirname(RbConfig.ruby)}#{File::PATH_SEPARATOR}#{ENV["PATH"]}" ENV.merge!("GEM_HOME" => nil, "GEM_PATH" => nil) # avoid installing gems to chruby-ed Ruby end @@ -52,7 +53,7 @@ def setup_cmds(c) def use_gemfile(extra_setup_cmd: nil) # Benchmarks should normally set their current directory and then call this method. - setup_cmds(["bundle check 2> /dev/null || bundle install", extra_setup_cmd].compact) + setup_cmds(["bundle check 2> #{File::NULL} || bundle install", extra_setup_cmd].compact) # Need to be in the appropriate directory for this... require "bundler" @@ -236,7 +237,7 @@ def write_json_file(ruby_bench_results) require "json" out_path = YB_OUTPUT_FILE - system('mkdir', '-p', File.dirname(out_path)) + FileUtils.mkdir_p(File.dirname(out_path)) # Using default path? Print where we put it. puts "Writing file #{out_path}" unless ENV["RESULT_JSON_PATH"] diff --git a/harness/harness.rb b/harness/harness.rb index 2fac8c3f..f6bd6099 100644 --- a/harness/harness.rb +++ b/harness/harness.rb @@ -14,7 +14,7 @@ RSS_CSV_PATH = ENV['RSS_CSV_PATH'] ? File.expand_path(ENV['RSS_CSV_PATH']) : nil -system('mkdir', '-p', File.dirname(OUT_CSV_PATH)) +FileUtils.mkdir_p(File.dirname(OUT_CSV_PATH)) # We could include other values in this result if more become relevant # but for now all we want to know is if YJIT was enabled at runtime. diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index 38f9222e..6d050092 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -3,6 +3,7 @@ require 'csv' require 'json' require 'rbconfig' +require 'shellwords' require_relative 'table_formatter' # Extracted helper methods from run_benchmarks.rb for testing @@ -116,21 +117,23 @@ def render_graph(json_path) GraphRenderer.render(json_path, png_path) end - # Checked system - error or return info if the command fails + # Checked system - error or return info if the command fails. + # An Array command with arguments runs without a shell, so Shellwords escapes never reach the program. def check_call(command, env: {}, raise_error: true, quiet: ENV['BENCHMARK_QUIET'] == '1') - puts("+ #{command}") unless quiet + command_str = command.is_a?(Array) ? command.shelljoin : command + puts("+ #{command_str}") unless quiet result = {} if quiet - result[:success] = system(env, command, out: File::NULL, err: File::NULL) + result[:success] = system(env, *command, out: File::NULL, err: File::NULL) else - result[:success] = system(env, command) + result[:success] = system(env, *command) end result[:status] = $? unless result[:success] - puts "Command #{command.inspect} failed with exit code #{result[:status].exitstatus} in directory #{Dir.pwd}" unless quiet + puts "Command #{command_str.inspect} failed with exit code #{result[:status].exitstatus} in directory #{Dir.pwd}" unless quiet raise RuntimeError.new if raise_error end diff --git a/lib/benchmark_runner/cli.rb b/lib/benchmark_runner/cli.rb index ed59d2d1..8c8e2ff5 100644 --- a/lib/benchmark_runner/cli.rb +++ b/lib/benchmark_runner/cli.rb @@ -47,7 +47,7 @@ def run # Collect ruby version descriptions for all executables upfront args.executables.each do |name, executable| - ruby_descriptions[name] = `#{executable.shelljoin} -v`.chomp + ruby_descriptions[name] = IO.popen([*executable, "-v"], &:read).chomp end # Warn if two executables look identical (same ruby -v output and same flags) diff --git a/lib/benchmark_suite.rb b/lib/benchmark_suite.rb index 4c22a12a..7599152a 100644 --- a/lib/benchmark_suite.rb +++ b/lib/benchmark_suite.rb @@ -161,15 +161,15 @@ def run_single_benchmark(script_path, result_json_path, ruby, cmd_prefix, env, b ENV["RESULT_JSON_PATH"] = result_json_path # Set up the benchmarking command - cmd = cmd_prefix + [ + cmd = (cmd_prefix + [ *ruby, "-I", benchmark_harness, *pre_init, script_path, - ].compact + ].compact).map(&:to_s) # Do the benchmarking - result = BenchmarkRunner.check_call(cmd.shelljoin, env: env, raise_error: false, quiet: quiet) + result = BenchmarkRunner.check_call(cmd, env: env, raise_error: false, quiet: quiet) result[:command] = cmd.shelljoin result ensure @@ -198,10 +198,10 @@ def compute_benchmark_env(ruby) # like `bundle install` in a child process will not use the Ruby being benchmarked. # It overrides PATH to guarantee the commands of the benchmarked Ruby will be used. env = {} - ruby_path = `#{ruby.shelljoin} -e 'print RbConfig.ruby' 2> #{File::NULL}` + ruby_path = IO.popen([*ruby, "-e", "print RbConfig.ruby"], err: File::NULL, &:read) if ruby_path != RbConfig.ruby - env["PATH"] = "#{File.dirname(ruby_path)}:#{ENV["PATH"]}" + env["PATH"] = "#{File.dirname(ruby_path)}#{File::PATH_SEPARATOR}#{ENV["PATH"]}" # chruby sets GEM_HOME and GEM_PATH in your shell. We have to unset it in the child # process to avoid installing gems to the version that is running run_benchmarks.rb.