Merge pull request #521 from eightbitraptor/mvh-ractor-harness-improvements

Improve Ractor harness summary output

Author: eightbitraptor

.github/workflows/test.yml

@@ -6,6 +6,8 @@ on:
   pull_request:
   schedule:
     - cron: "00 15 * * *"
+env:
+  BUNDLE_VERSION: system
 jobs:
   test:
     runs-on: ubuntu-latest

harness-ractor/harness.rb

@@ -73,11 +73,11 @@ def run_benchmark(num_itrs_hint, ractor_args: [], &block)
       time = Process.clock_gettime(Process::CLOCK_MONOTONIC) - before
       time_ms = (1000 * time).to_i
       itr_str = "%-3s %4s %6s" % ["#{rs}", "##{num_itrs}:", "#{time_ms}ms"]
-      stats[rs] << time_ms
+      stats[rs] << time
       puts itr_str
     end
   end
-  return_results([], stats.values.flatten)
+  return_results([], stats.values.flatten, bench_by_ractors: stats)
 end
 
 # NOTE: we use `ractor_deep_dup` instead of `Ractor.make_shareable(copy: true)` for the case of

lib/benchmark_runner.rb

@@ -48,7 +48,7 @@ def write_csv(output_path, ruby_descriptions, table)
     end
 
     # Build output text string with metadata, table, and legend
-    def build_output_text(ruby_descriptions, table, format, bench_failures, include_rss: false, include_gc: false, include_pvalue: false, gc_table: nil, gc_format: nil)
+    def build_output_text(ruby_descriptions, table, format, bench_failures, include_rss: false, include_gc: false, include_pvalue: false, gc_table: nil, gc_format: nil, sections: nil)
       base_name, *other_names = ruby_descriptions.keys
 
       output_str = +""
@@ -58,11 +58,17 @@ def build_output_text(ruby_descriptions, table, format, bench_failures, include_
       end
 
       output_str << "\n"
-      output_str << TableFormatter.new(table, format, bench_failures).to_s + "\n"
-
-      if include_gc && gc_table && gc_format
-        output_str << "GC summary:\n"
-        output_str << TableFormatter.new(gc_table, gc_format, {}).to_s + "\n"
+      sections ||= [{ table: table, format: format, failures: bench_failures, include_gc: include_gc, gc_table: gc_table, gc_format: gc_format }]
+      has_gc_summary = sections.any? { |section| section[:include_gc] && section[:gc_table] }
+      sections.each do |section|
+        title = section[:title]
+        output_str << "#{title}:\n" if title
+        output_str << TableFormatter.new(section[:table], section[:format], section.fetch(:failures, {})).to_s + "\n"
+
+        if section[:include_gc] && section[:gc_table] && section[:gc_format]
+          output_str << (title ? "GC summary (#{title}):\n" : "GC summary:\n")
+          output_str << TableFormatter.new(section[:gc_table], section[:gc_format], {}).to_s + "\n"
+        end
       end
 
       unless other_names.empty?
@@ -74,7 +80,7 @@ def build_output_text(ruby_descriptions, table, format, bench_failures, include_
             output_str << "- RSS #{base_name}/#{name}: ratio of #{base_name}/#{name} RSS. Higher is better for #{name}. Above 1 means lower memory usage.\n"
           end
         end
-        if include_gc && gc_table
+        if has_gc_summary
           output_str << "- GC summary compares #{base_name} → comparison. Ratio columns are #{base_name}/comparison; above 1 means the comparison spent less GC time.\n"
           output_str << "- mark/iter ratio and sweep/iter ratio compare total GC phase time per benchmark iteration, so they include both per-GC cost and GC frequency changes.\n"
           output_str << "- mark/GC ratio and sweep/GC ratio compare average phase time per GC, isolating whether each GC became cheaper or more expensive.\n"

lib/benchmark_runner/cli.rb

@@ -6,6 +6,8 @@
 require_relative '../benchmark_runner'
 require_relative '../benchmark_suite'
 require_relative '../results_table_builder'
+require_relative '../ractor_breakdown'
+require_relative '../row_layout'
 
 module BenchmarkRunner
   class CLI
@@ -63,6 +65,9 @@ def run
       bench_start_time = Time.now.to_f
       bench_data = {}
       bench_failures = {}
+      bench_harnesses = suite.benchmarks.each_with_object({}) do |entry, h|
+        h[entry.name] = suite.harness_for(entry.name)
+      end
 
       if args.interleave
         args.executables.each_key { |name| bench_data[name] = {} }
@@ -104,7 +109,7 @@ def run
 
       puts
 
-      # Build results table
+      # Build the results table
       builder = ResultsTableBuilder.new(
         executable_names: ruby_descriptions.keys,
         bench_data: bench_data,
@@ -125,7 +130,8 @@ def run
       BenchmarkRunner.write_csv(output_path, ruby_descriptions, table)
 
       # Save the output in a text file that we can easily refer to
-      output_str = BenchmarkRunner.build_output_text(ruby_descriptions, table, format, bench_failures, include_rss: args.rss, include_gc: builder.include_gc?, include_pvalue: args.pvalue, gc_table: gc_table, gc_format: gc_format)
+      output_sections = build_output_sections(ruby_descriptions.keys, bench_data, bench_harnesses, bench_failures)
+      output_str = BenchmarkRunner.build_output_text(ruby_descriptions, table, format, bench_failures, include_rss: args.rss, include_gc: builder.include_gc?, include_pvalue: args.pvalue, gc_table: gc_table, gc_format: gc_format, sections: output_sections)
       out_txt_path = output_path + ".txt"
       File.open(out_txt_path, "w") { |f| f.write output_str }
 
@@ -149,5 +155,81 @@ def run
         exit(1)
       end
     end
+
+    private
+
+    def build_output_sections(executable_names, bench_data, bench_harnesses, bench_failures)
+      ordered_names = sorted_benchmark_names(executable_names, bench_data)
+      failed_names = bench_failures.values.flat_map(&:keys).uniq
+      ordered_names.concat(failed_names.reject { |name| ordered_names.include?(name) })
+
+      names_by_harness = {}
+      ordered_names.each do |bench_name|
+        harness = bench_harnesses.fetch(bench_name, args.harness)
+        names_by_harness[harness] ||= []
+        names_by_harness[harness] << bench_name
+      end
+
+      show_titles = names_by_harness.size > 1
+      names_by_harness.map do |harness, names|
+        section = build_output_section(executable_names, bench_data, bench_failures, harness, names)
+        section[:title] = nil unless show_titles
+        section
+      end
+    end
+
+    def build_output_section(executable_names, bench_data, bench_failures, harness, bench_names)
+      section_data = slice_bench_data(bench_data, bench_names)
+      breakdown = RactorBreakdown.expand(section_data)
+      use_ractor_layout = harness == BenchmarkSuite::RACTOR_HARNESS && !breakdown.groups.empty?
+      layout = use_ractor_layout ? RactorRowLayout.new(groups: breakdown.groups) : FlatRowLayout.new
+      display_data = use_ractor_layout ? breakdown.bench_data : section_data
+
+      builder = ResultsTableBuilder.new(
+        executable_names: executable_names,
+        bench_data: display_data,
+        include_rss: args.rss,
+        include_pvalue: args.pvalue,
+        zjit_stats: args.zjit_stats,
+        row_layout: layout
+      )
+      table, format, gc_table, gc_format = builder.build
+
+      {
+        title: harness,
+        table: table,
+        format: format,
+        failures: slice_failures(bench_failures, bench_names),
+        include_gc: builder.include_gc?,
+        gc_table: gc_table,
+        gc_format: gc_format,
+      }
+    end
+
+    def sorted_benchmark_names(executable_names, bench_data)
+      builder = ResultsTableBuilder.new(
+        executable_names: executable_names,
+        bench_data: bench_data,
+        include_rss: args.rss,
+        include_pvalue: args.pvalue,
+        zjit_stats: args.zjit_stats
+      )
+      builder.bench_names
+    end
+
+    def slice_bench_data(bench_data, bench_names)
+      wanted = bench_names.each_with_object({}) { |name, h| h[name] = true }
+      bench_data.each_with_object({}) do |(executable, benchmarks), sliced|
+        sliced[executable] = benchmarks.select { |name, _data| wanted[name] }
+      end
+    end
+
+    def slice_failures(bench_failures, bench_names)
+      wanted = bench_names.each_with_object({}) { |name, h| h[name] = true }
+      bench_failures.each_with_object({}) do |(executable, failures), sliced|
+        selected = failures.select { |name, _failure| wanted[name] }
+        sliced[executable] = selected unless selected.empty?
+      end
+    end
   end
 end

lib/benchmark_suite.rb

@@ -38,6 +38,10 @@ def benchmarks
     @benchmarks ||= discover_benchmarks
   end
 
+  def harness_for(benchmark_name)
+    benchmark_harness_for(benchmark_name)
+  end
+
   # Run a single benchmark entry on a single executable.
   # Returns { name:, data: } on success, { name:, failure: } on error.
   def run_benchmark(entry, ruby:, ruby_description:)
@@ -50,19 +54,21 @@ def run_benchmark(entry, ruby:, ruby_description:)
 
     # Clear project-level Bundler environment so benchmarks run in a clean context.
     # Benchmarks that need Bundler (e.g., railsbench) set up their own via use_gemfile.
+    benchmark_harness = benchmark_harness_for(entry.name)
+
     result = if defined?(Bundler)
       Bundler.with_unbundled_env do
-        run_single_benchmark(entry.script_path, result_json_path, ruby, cmd_prefix, env, entry.name, quiet: quiet)
+        run_single_benchmark(entry.script_path, result_json_path, ruby, cmd_prefix, env, benchmark_harness, quiet: quiet)
       end
     else
-      run_single_benchmark(entry.script_path, result_json_path, ruby, cmd_prefix, env, entry.name, quiet: quiet)
+      run_single_benchmark(entry.script_path, result_json_path, ruby, cmd_prefix, env, benchmark_harness, quiet: quiet)
     end
 
     if result[:success]
-      { name: entry.name, data: process_benchmark_result(result_json_path, result[:command], delete_file: !caller_json_path) }
+      { name: entry.name, data: process_benchmark_result(result_json_path, result[:command], delete_file: !caller_json_path), harness: benchmark_harness }
     else
       FileUtils.rm_f(result_json_path) unless caller_json_path
-      { name: entry.name, failure: result[:status].exitstatus }
+      { name: entry.name, failure: result[:status].exitstatus, harness: benchmark_harness }
     end
   end
 
@@ -145,7 +151,7 @@ def filter_entries(entries, categories:, name_filters:, excludes:, directory_map
     entries.select { |entry| filter.match?(entry.name) }
   end
 
-  def run_single_benchmark(script_path, result_json_path, ruby, cmd_prefix, env, benchmark_name, quiet: false)
+  def run_single_benchmark(script_path, result_json_path, ruby, cmd_prefix, env, benchmark_harness, quiet: false)
     # Fix for jruby/jruby#7394 in JRuby 9.4.2.0
     script_path = File.expand_path(script_path)
 
@@ -154,9 +160,6 @@ def run_single_benchmark(script_path, result_json_path, ruby, cmd_prefix, env, b
     original_result_json_path = ENV["RESULT_JSON_PATH"]
     ENV["RESULT_JSON_PATH"] = result_json_path
 
-    # Use per-benchmark default_harness if set, otherwise use global harness
-    benchmark_harness = benchmark_harness_for(benchmark_name)
-
     # Set up the benchmarking command
     cmd = cmd_prefix + [
       *ruby,

lib/ractor_breakdown.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module RactorBreakdown
+  KEY_SEP = "\x00"
+
+  Result = Struct.new(:bench_data, :groups)
+
+  module_function
+
+  def data_key(base_name, count)
+    "#{base_name}#{KEY_SEP}#{count}"
+  end
+
+  def base_name(data_key)
+    data_key.split(KEY_SEP, 2).first
+  end
+
+  def expand(bench_data)
+    groups = {}
+    new_data = {}
+
+    bench_data.each do |exe, benchmarks|
+      new_data[exe] = {}
+      benchmarks.each do |name, blob|
+        breakdown = blob.is_a?(Hash) && blob['bench_by_ractors']
+        unless breakdown
+          new_data[exe][name] = blob
+          next
+        end
+
+        counts = breakdown.keys.map { |c| Integer(c) }.sort
+        groups[name] ||= counts.map { |c| [data_key(name, c), c] }
+
+        counts.each do |count|
+          key = data_key(name, count)
+          new_data[exe][key] = per_count_blob(blob, breakdown, count)
+        end
+      end
+    end
+
+    Result.new(new_data, groups.to_a)
+  end
+
+  def per_count_blob(blob, breakdown, count)
+    per_count = blob.reject { |k, _| k == 'bench_by_ractors' || k == 'bench' }
+    per_count['bench'] = breakdown[count.to_s]
+    per_count['warmup'] = []
+    per_count
+  end
+end

lib/results_table_builder.rb

@@ -1,11 +1,14 @@
 require_relative '../misc/stats'
+require_relative 'row_layout'
 require 'yaml'
 
 class ResultsTableBuilder
   SECONDS_TO_MS = 1000.0
   BYTES_TO_MIB = 1024.0 * 1024.0
 
-  def initialize(executable_names:, bench_data:, include_rss: false, include_pvalue: false, zjit_stats: [])
+  attr_reader :bench_names
+
+  def initialize(executable_names:, bench_data:, include_rss: false, include_pvalue: false, zjit_stats: [], row_layout: FlatRowLayout.new)
     @executable_names = executable_names
     @bench_data = bench_data
     @include_rss = include_rss
@@ -15,6 +18,7 @@ def initialize(executable_names:, bench_data:, include_rss: false, include_pvalu
     @rss_has_samples = @include_rss && detect_rss_samples(bench_data)
     @base_name = executable_names.first
     @other_names = executable_names[1..]
+    @row_layout = row_layout
     @bench_names = compute_bench_names
   end
 
@@ -26,11 +30,10 @@ def build
     table = [build_header]
     format = build_format
 
-    @bench_names.each do |bench_name|
-      next unless has_complete_data?(bench_name)
+    @row_layout.entries(@bench_names).each do |entry|
+      next unless has_complete_data?(entry.data_key)
 
-      row = build_row(bench_name)
-      table << row
+      table << (entry.label_cells + build_stat_cells(entry.data_key))
     end
 
     gc_table = build_gc_summary_table
@@ -45,7 +48,7 @@ def has_complete_data?(bench_name)
   end
 
   def build_header
-    header = ["bench"]
+    header = ["bench", *@row_layout.extra_header_columns]
 
     @executable_names.each do |name|
       header << "#{name} (ms)"
@@ -74,7 +77,7 @@ def build_header
   end
 
   def build_format
-    format = ["%s"]
+    format = ["%s", *@row_layout.extra_format_columns]
 
     @executable_names.each do |_name|
       format << "%s"
@@ -135,7 +138,7 @@ def build_gc_summary_format(gc_table)
     Array.new(gc_table.first.size, "%s")
   end
 
-  def build_row(bench_name)
+  def build_stat_cells(bench_name)
     t0s = extract_first_iteration_times(bench_name)
     times_no_warmup = extract_benchmark_times(bench_name)
     rsss = extract_rss_values(bench_name)
@@ -153,7 +156,7 @@ def build_row(bench_name)
       [stat, extract_zjit_stat(bench_name, stat)]
     end
 
-    row = [bench_name]
+    row = []
     build_base_columns(row, base_t, base_rss_cell, zjit_stat_values, 0)
     build_comparison_columns(row, other_ts, other_rss_cells, zjit_stat_values)
     build_ratio_columns(row, base_t0, other_t0s, base_t, other_ts)
@@ -438,7 +441,7 @@ def sort_benchmarks(bench_names, metadata)
   end
 
   def category_priority(bench_name, metadata)
-    category = metadata.dig(bench_name, 'category') || 'other'
+    category = metadata.dig(@row_layout.base_name(bench_name), 'category') || 'other'
     case category
     when 'headline' then 0
     when 'micro' then 2