Inline method in the only object that is using it

Author: rafaelfranca

lib/benchmark_runner.rb

@@ -16,16 +16,6 @@ def free_file_no(directory)
     end
   end
 
-  # Sort benchmarks with headlines first, then others, then micro
-  def sort_benchmarks(bench_names, metadata)
-    headline_benchmarks = metadata.select { |_, meta| meta['category'] == 'headline' }.keys
-    micro_benchmarks = metadata.select { |_, meta| meta['category'] == 'micro' }.keys
-
-    headline_names, bench_names = bench_names.partition { |name| headline_benchmarks.include?(name) }
-    micro_names, other_names = bench_names.partition { |name| micro_benchmarks.include?(name) }
-    headline_names.sort + other_names.sort + micro_names.sort
-  end
-
   # Checked system - error or return info if the command fails
   def check_call(command, env: {}, raise_error: true, quiet: false)
     puts("+ #{command}") unless quiet

lib/results_table_builder.rb

@@ -1,16 +1,17 @@
 require_relative '../misc/stats'
+require 'yaml'
 
 class ResultsTableBuilder
   SECONDS_TO_MS = 1000.0
   BYTES_TO_MIB = 1024.0 * 1024.0
 
-  def initialize(executable_names:, bench_data:, bench_names:, include_rss: false)
+  def initialize(executable_names:, bench_data:, include_rss: false)
     @executable_names = executable_names
     @bench_data = bench_data
-    @bench_names = bench_names
     @include_rss = include_rss
     @base_name = executable_names.first
     @other_names = executable_names[1..]
+    @bench_names = compute_bench_names
   end
 
   def build
@@ -143,4 +144,21 @@ def stddev(values)
   def stddev_percent(values)
     100 * stddev(values) / mean(values)
   end
+
+  def compute_bench_names
+    # Get keys from all rows in case a benchmark failed for only some executables
+    all_bench_names = @bench_data.map { |k, v| v.keys }.flatten.uniq
+    benchmarks_metadata = YAML.load_file('benchmarks.yml')
+    sort_benchmarks(all_bench_names, benchmarks_metadata)
+  end
+
+  # Sort benchmarks with headlines first, then others, then micro
+  def sort_benchmarks(bench_names, metadata)
+    headline_benchmarks = metadata.select { |_, meta| meta['category'] == 'headline' }.keys
+    micro_benchmarks = metadata.select { |_, meta| meta['category'] == 'micro' }.keys
+
+    headline_names, bench_names = bench_names.partition { |name| headline_benchmarks.include?(name) }
+    micro_names, other_names = bench_names.partition { |name| micro_benchmarks.include?(name) }
+    headline_names.sort + other_names.sort + micro_names.sort
+  end
 end

run_benchmarks.rb

@@ -15,11 +15,6 @@
 require_relative 'lib/argument_parser'
 require_relative 'lib/results_table_builder'
 
-def sort_benchmarks(bench_names)
-  benchmarks_metadata = YAML.load_file('benchmarks.yml')
-  BenchmarkRunner.sort_benchmarks(bench_names, benchmarks_metadata)
-end
-
 args = ArgumentParser.parse(ARGV)
 
 CPUConfig.configure_for_benchmarking(turbo: args.turbo)
@@ -52,9 +47,6 @@ def sort_benchmarks(bench_names)
 end
 
 bench_end_time = Time.now.to_f
-# Get keys from all rows in case a benchmark failed for only some executables.
-bench_names = sort_benchmarks(bench_data.map { |k, v| v.keys }.flatten.uniq)
-
 bench_total_time = (bench_end_time - bench_start_time).to_i
 puts("Total time spent benchmarking: #{bench_total_time}s")
 
@@ -70,7 +62,6 @@ def sort_benchmarks(bench_names)
 builder = ResultsTableBuilder.new(
   executable_names: all_names,
   bench_data: bench_data,
-  bench_names: bench_names,
   include_rss: args.rss
 )
 table, format = builder.build

test/benchmark_runner_test.rb

@@ -49,57 +49,6 @@
     end
   end
 
-  describe '.sort_benchmarks' do
-    before do
-      @metadata = {
-        'fib' => { 'category' => 'micro' },
-        'railsbench' => { 'category' => 'headline' },
-        'optcarrot' => { 'category' => 'headline' },
-        'some_bench' => { 'category' => 'other' },
-        'another_bench' => { 'category' => 'other' },
-        'zebra' => { 'category' => 'other' }
-      }
-    end
-
-    it 'sorts benchmarks with headlines first, then others, then micro' do
-      bench_names = ['fib', 'some_bench', 'railsbench', 'another_bench', 'optcarrot']
-      result = BenchmarkRunner.sort_benchmarks(bench_names, @metadata)
-
-      # Headlines should be first
-      headline_indices = [result.index('railsbench'), result.index('optcarrot')]
-      assert_equal true, headline_indices.all? { |i| i < 2 }
-
-      # Micro should be last
-      assert_equal 'fib', result.last
-
-      # Others in the middle
-      other_indices = [result.index('some_bench'), result.index('another_bench')]
-      assert_equal true, other_indices.all? { |i| i >= 2 && i < result.length - 1 }
-    end
-
-    it 'sorts alphabetically within categories' do
-      bench_names = ['zebra', 'another_bench', 'some_bench']
-      result = BenchmarkRunner.sort_benchmarks(bench_names, @metadata)
-      assert_equal ['another_bench', 'some_bench', 'zebra'], result
-    end
-
-    it 'handles empty list' do
-      result = BenchmarkRunner.sort_benchmarks([], @metadata)
-      assert_equal [], result
-    end
-
-    it 'handles single benchmark' do
-      result = BenchmarkRunner.sort_benchmarks(['fib'], @metadata)
-      assert_equal ['fib'], result
-    end
-
-    it 'handles only headline benchmarks' do
-      bench_names = ['railsbench', 'optcarrot']
-      result = BenchmarkRunner.sort_benchmarks(bench_names, @metadata)
-      assert_equal ['optcarrot', 'railsbench'], result
-    end
-  end
-
   describe '.check_call' do
     it 'runs a successful command and returns success status' do
       result = nil