Merge pull request #434 from ruby/rmf-extract-benchmark-filter

Extract BenchmarkFilter class to its own file

Author: rafaelfranca

lib/benchmark_filter.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+# Filters benchmarks based on categories and name patterns
+class BenchmarkFilter
+  def initialize(categories:, name_filters:, metadata:)
+    @categories = categories
+    @name_filters = process_name_filters(name_filters)
+    @metadata = metadata
+    @category_cache = {}
+  end
+
+  def match?(entry)
+    name = entry.sub(/\.rb\z/, '')
+    matches_category?(name) && matches_name_filter?(name)
+  end
+
+  private
+
+  def matches_category?(name)
+    return true if @categories.empty?
+
+    benchmark_categories = get_benchmark_categories(name)
+    @categories.intersect?(benchmark_categories)
+  end
+
+  def matches_name_filter?(name)
+    return true if @name_filters.empty?
+
+    @name_filters.any? { |filter| filter === name }
+  end
+
+  def get_benchmark_categories(name)
+    @category_cache[name] ||= begin
+      benchmark_metadata = @metadata[name] || {}
+      categories = [benchmark_metadata.fetch('category', 'other')]
+      categories << 'ractor' if benchmark_metadata['ractor']
+      categories
+    end
+  end
+
+  # Process "/my_benchmark/i" into /my_benchmark/i
+  def process_name_filters(name_filters)
+    name_filters.map do |name_filter|
+      if name_filter.start_with?("/")
+        parse_regexp_filter(name_filter)
+      else
+        name_filter
+      end
+    end
+  end
+
+  def parse_regexp_filter(filter)
+    regexp_str = filter[1..-1].reverse.sub(/\A(\w*)\//, "")
+    regexp_opts = ::Regexp.last_match(1).to_s
+    regexp_str.reverse!
+
+    return Regexp.new(regexp_str) if regexp_opts.empty?
+
+    # Convert option string to Regexp option flags
+    flags = 0
+    flags |= Regexp::IGNORECASE if regexp_opts.include?('i')
+    flags |= Regexp::MULTILINE if regexp_opts.include?('m')
+    flags |= Regexp::EXTENDED if regexp_opts.include?('x')
+
+    Regexp.new(regexp_str, flags)
+  end
+end

lib/benchmark_runner.rb

@@ -16,45 +16,6 @@ def free_file_no(directory)
     end
   end
 
-  # Get benchmark categories from metadata
-  def benchmark_categories(name, metadata)
-    benchmark_metadata = metadata[name] || {}
-    categories = [benchmark_metadata.fetch('category', 'other')]
-    categories << 'ractor' if benchmark_metadata['ractor']
-    categories
-  end
-
-  # Check if the name matches any of the names in a list of filters
-  def match_filter(entry, categories:, name_filters:, metadata:)
-    name_filters = process_name_filters(name_filters)
-    name = entry.sub(/\.rb\z/, '')
-    (categories.empty? || benchmark_categories(name, metadata).any? { |cat| categories.include?(cat) }) &&
-      (name_filters.empty? || name_filters.any? { |filter| filter === name })
-  end
-
-  # Process "/my_benchmark/i" into /my_benchmark/i
-  def process_name_filters(name_filters)
-    name_filters.map do |name_filter|
-      if name_filter[0] == "/"
-        regexp_str = name_filter[1..-1].reverse.sub(/\A(\w*)\//, "")
-        regexp_opts = ::Regexp.last_match(1).to_s
-        regexp_str.reverse!
-        r = /#{regexp_str}/
-        if !regexp_opts.empty?
-          # Convert option string to Regexp option flags
-          flags = 0
-          flags |= Regexp::IGNORECASE if regexp_opts.include?('i')
-          flags |= Regexp::MULTILINE if regexp_opts.include?('m')
-          flags |= Regexp::EXTENDED if regexp_opts.include?('x')
-          r = Regexp.new(regexp_str, flags)
-        end
-        r
-      else
-        name_filter
-      end
-    end
-  end
-
   # Resolve the pre_init file path into a form that can be required
   def expand_pre_init(path)
     require 'pathname'

run_benchmarks.rb

@@ -13,6 +13,7 @@
 require_relative 'misc/stats'
 require_relative 'lib/benchmark_runner'
 require_relative 'lib/table_formatter'
+require_relative 'lib/benchmark_filter'
 
 # Checked system - error or return info if the command fails
 def check_call(command, env: {}, raise_error: true, quiet: false)
@@ -113,9 +114,14 @@ def stddev(values)
   Stats.new(values).stddev
 end
 
-# Check if the name matches any of the names in a list of filters
-def match_filter(entry, categories:, name_filters:)
-  BenchmarkRunner.match_filter(entry, categories: categories, name_filters: name_filters, metadata: benchmarks_metadata)
+def benchmark_filter(categories:, name_filters:)
+  @benchmark_filter ||= {}
+  key = [categories, name_filters]
+  @benchmark_filter[key] ||= BenchmarkFilter.new(
+    categories: categories,
+    name_filters: name_filters,
+    metadata: benchmarks_metadata
+  )
 end
 
 def benchmarks_metadata
@@ -147,16 +153,18 @@ def run_benchmarks(ruby:, ruby_description:, categories:, name_filters:, out_pat
   bench_file_grouping = {}
 
   # Get the list of benchmark files/directories matching name filters
+  filter = benchmark_filter(categories: categories, name_filters: name_filters)
   bench_file_grouping[bench_dir] = Dir.children(bench_dir).sort.filter do |entry|
-    match_filter(entry, categories: categories, name_filters: name_filters)
+    filter.match?(entry)
   end
 
   if categories == ["ractor"]
     # We ignore the category filter here because everything in the
     # benchmarks-ractor directory should be included when we're benchmarking the
     # Ractor category
+    ractor_filter = benchmark_filter(categories: [], name_filters: name_filters)
     bench_file_grouping[ractor_bench_dir] = Dir.children(ractor_bench_dir).sort.filter do |entry|
-      match_filter(entry, categories: [], name_filters: name_filters)
+      ractor_filter.match?(entry)
     end
   end
 

test/benchmark_filter_test.rb

@@ -0,0 +1,90 @@
+require_relative 'test_helper'
+require_relative '../lib/benchmark_filter'
+require_relative '../lib/benchmark_runner'
+
+describe BenchmarkFilter do
+  before do
+    @metadata = {
+      'fib' => { 'category' => 'micro' },
+      'railsbench' => { 'category' => 'headline' },
+      'optcarrot' => { 'category' => 'headline' },
+      'ractor_bench' => { 'category' => 'other', 'ractor' => true }
+    }
+  end
+
+  describe '#match?' do
+    it 'matches when no filters provided' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: [], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+    end
+
+    it 'matches by category' do
+      filter = BenchmarkFilter.new(categories: ['micro'], name_filters: [], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+      assert_equal false, filter.match?('railsbench.rb')
+    end
+
+    it 'matches by name filter' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['fib'], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+      assert_equal false, filter.match?('railsbench.rb')
+    end
+
+    it 'matches ractor category' do
+      filter = BenchmarkFilter.new(categories: ['ractor'], name_filters: [], metadata: @metadata)
+
+      assert_equal true, filter.match?('ractor_bench.rb')
+    end
+
+    it 'strips .rb extension from entry name' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['fib'], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+    end
+
+    it 'handles regex filters' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['/rails/'], metadata: @metadata)
+
+      assert_equal true, filter.match?('railsbench.rb')
+      assert_equal false, filter.match?('fib.rb')
+    end
+
+    it 'handles case-insensitive regex filters' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['/RAILS/i'], metadata: @metadata)
+
+      assert_equal true, filter.match?('railsbench.rb')
+    end
+
+    it 'handles multiple categories' do
+      filter = BenchmarkFilter.new(categories: ['micro', 'headline'], name_filters: [], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+      assert_equal true, filter.match?('railsbench.rb')
+    end
+
+    it 'requires both category and name filter to match when both provided' do
+      filter = BenchmarkFilter.new(categories: ['micro'], name_filters: ['rails'], metadata: @metadata)
+
+      assert_equal false, filter.match?('fib.rb') # matches category but not name
+      assert_equal false, filter.match?('railsbench.rb') # matches name but not category
+    end
+
+    it 'handles complex regex patterns' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['/opt.*rot/'], metadata: @metadata)
+
+      assert_equal true, filter.match?('optcarrot.rb')
+      assert_equal false, filter.match?('fib.rb')
+    end
+
+    it 'handles mixed string and regex filters' do
+      filter = BenchmarkFilter.new(categories: [], name_filters: ['fib', '/rails/'], metadata: @metadata)
+
+      assert_equal true, filter.match?('fib.rb')
+      assert_equal true, filter.match?('railsbench.rb')
+      assert_equal false, filter.match?('optcarrot.rb')
+    end
+  end
+end