Merge pull request #1382 from mbj/optimize-parsing

Significantly optimize parsing overhead when `--since` is specified

Changelog.md

@@ -1,5 +1,9 @@
 # v0.11.20 unreleased
 
+* [#1382](https://github.com/mbj/mutant/pull/1382)
+
+  Significantly optimize parsing overhead during boot. Smaller projects may see boot time speedups of 10-20%. Larger projects may see boot time improvements of 2-3x or greater. Memory consumption is also greatly reduced (by a factor of 5x or more in some small changesets).
+
 * [#1378](https://github.com/mbj/mutant/pull/1379)
 
   Add support for user defined mutation worker process hooks. 

lib/mutant/cli/command/environment.rb

@@ -92,7 +92,6 @@ def add_integration_options(parser)
           end
         end
 
-        # rubocop:disable Metrics/MethodLength
         def add_matcher_options(parser)
           parser.separator('Matcher:')
 
@@ -103,10 +102,7 @@ def add_matcher_options(parser)
             add_matcher(:start_expressions, @config.expression_parser.call(pattern).from_right)
           end
           parser.on('--since REVISION', 'Only select subjects touched since REVISION') do |revision|
-            add_matcher(
-              :subject_filters,
-              Repository::SubjectFilter.new(diff: Repository::Diff.new(to: revision, world: world))
-            )
+            add_matcher(:diffs, Repository::Diff.new(to: revision, world: world))
           end
         end
 

lib/mutant/matcher.rb

@@ -31,7 +31,9 @@ def self.allowed_subject?(config, subject)
     private_class_method :allowed_subject?
 
     def self.select_subject?(config, subject)
-      config.subject_filters.all? { |filter| filter.call(subject) }
+      config.diffs.all? do |diff|
+        diff.touches?(subject.source_path, subject.source_lines)
+      end
     end
     private_class_method :select_subject?
 

lib/mutant/matcher/config.rb

@@ -8,7 +8,7 @@ class Config
         :ignore,
         :subjects,
         :start_expressions,
-        :subject_filters
+        :diffs
       )
 
       INSPECT_FORMAT      = "#<#{self} %s>"
@@ -19,8 +19,8 @@ class Config
       PRESENTATIONS       = {
         ignore:            :syntax,
         start_expressions: :syntax,
-        subject_filters:   :inspect,
-        subjects:          :syntax
+        subjects:          :syntax,
+        diffs:             :inspect
       }.freeze
 
       private_constant(*constants(false))

lib/mutant/matcher/method.rb

@@ -120,13 +120,22 @@ def subject_config(node)
         def matched_view
           return if source_location.nil?
 
+          # This is a performance optimization when using --since to avoid the cost of parsing
+          # every source file that could possibly map to a subject. A more fine-grained filtering
+          # takes places later in the process.
+          return unless relevant_source_file?
+
           ast
             .on_line(source_line)
             .select { |view| view.node.type.eql?(self.class::MATCH_NODE_TYPE) && match?(view.node) }
             .last
         end
         memoize :matched_view
 
+        def relevant_source_file?
+          env.config.matcher.diffs.all? { |diff| diff.touches_path?(source_path) }
+        end
+
         def visibility
           # This can be cleaned up once we are on >ruby-3.0
           # Method#{public,private,protected}? exists there.

lib/mutant/repository.rb

@@ -2,19 +2,5 @@
 
 module Mutant
   module Repository
-    # Subject filter based on repository diff
-    class SubjectFilter
-      include Adamantium, Anima.new(:diff)
-
-      # Test if subject was touched in diff
-      #
-      # @param [Subject] subject
-      #
-      # @return [Boolean]
-      def call(subject)
-        diff.touches?(subject.source_path, subject.source_lines)
-      end
-
-    end # SubjectFilter
   end # Repository
 end # Mutant

spec/unit/mutant/cli_spec.rb

@@ -1298,33 +1298,15 @@ def self.main_body
           let(:arguments) { super() + ['--since', 'reference'] }
 
           let(:load_config_file_config) do
-            super().with(
-              matcher: file_config.matcher.with(
-                subject_filters: [
-                  Mutant::Repository::SubjectFilter.new(
-                    diff: Mutant::Repository::Diff.new(
-                      to:    'reference',
-                      world: world
-                    )
-                  )
-                ]
-              )
-            )
+            diff = Mutant::Repository::Diff.new(to: 'reference', world: world)
+
+            super().with(matcher: file_config.matcher.with(diffs: [diff]))
           end
 
           let(:bootstrap_config) do
-            super().with(
-              matcher: file_config.matcher.with(
-                subject_filters: [
-                  Mutant::Repository::SubjectFilter.new(
-                    diff: Mutant::Repository::Diff.new(
-                      to:    'reference',
-                      world: world
-                    )
-                  )
-                ]
-              )
-            )
+            diff = Mutant::Repository::Diff.new(to: 'reference', world: world)
+
+            super().with(matcher: file_config.matcher.with(diffs: [diff]))
           end
 
           include_examples 'CLI run'

spec/unit/mutant/matcher/config_spec.rb

@@ -13,17 +13,17 @@ def apply
       described_class.new(
         ignore:            [parse_expression('Ignore#a')],
         start_expressions: [parse_expression('Start#a')],
-        subject_filters:   [proc_a],
-        subjects:          [parse_expression('Match#a')]
+        subjects:          [parse_expression('Match#a')],
+        diffs:             []
       )
     end
 
     let(:other) do
       described_class.new(
         ignore:            [parse_expression('Ignore#b')],
         start_expressions: [parse_expression('Start#b')],
-        subject_filters:   [proc_b],
-        subjects:          [parse_expression('Match#b')]
+        subjects:          [parse_expression('Match#b')],
+        diffs:             []
       )
     end
 
@@ -32,8 +32,8 @@ def apply
         described_class.new(
           ignore:            [parse_expression('Ignore#a'), parse_expression('Ignore#b')],
           start_expressions: [parse_expression('Start#a'), parse_expression('Start#b')],
-          subject_filters:   [proc_a, proc_b],
-          subjects:          [parse_expression('Match#a'), parse_expression('Match#b')]
+          subjects:          [parse_expression('Match#a'), parse_expression('Match#b')],
+          diffs:             []
         )
       )
     end
@@ -72,14 +72,5 @@ def apply
 
       it { should eql('#<Mutant::Matcher::Config ignore: [Bar] subjects: [Foo]>') }
     end
-
-    context 'with subject filter' do
-      let(:object) do
-        described_class::DEFAULT
-          .add(:subject_filters, 'foo')
-      end
-
-      it { should eql('#<Mutant::Matcher::Config subject_filters: ["foo"]>') }
-    end
   end
 end

spec/unit/mutant/matcher/method/instance_spec.rb

@@ -10,6 +10,7 @@
   let(:object)        { described_class.new(scope: scope, target_method: method) }
   let(:source_path)   { MutantSpec::ROOT.join('test_app/lib/test_app.rb')        }
   let(:type)          { :def                                                     }
+  let(:config)        { Mutant::Config::DEFAULT                                  }
 
   let(:world) do
     instance_double(
@@ -21,7 +22,7 @@
   let(:env) do
     instance_double(
       Mutant::Env,
-      config: Mutant::Config::DEFAULT,
+      config: config,
       parser: Fixtures::TEST_ENV.parser,
       world:  world
     )
@@ -56,9 +57,62 @@ def arguments
     end
 
     include_examples 'skipped candidate'
+  end
+
+  context 'when method is defined inside of a file removed by a diff filter' do
+    let(:config)            { super().with(matcher: super().matcher.with(diffs: diffs)) }
+    let(:diff_a)            { instance_double(Mutant::Repository::Diff, :diff_a)        }
+    let(:diff_a_touches?)   { false                                                     }
+    let(:diff_b)            { instance_double(Mutant::Repository::Diff, :diff_b)        }
+    let(:diff_b_touches?)   { false                                                     }
+    let(:diffs)             { []                                                        }
+    let(:expected_warnings) { []                                                        }
+    let(:method_line)       { 13                                                        }
+    let(:method_name)       { :bar                                                      }
+    let(:scope)             { base::WithMemoizer                                        }
+
+    before do
+      allow(diff_a).to receive(:touches_path?).with(source_path).and_return(diff_a_touches?)
+      allow(diff_b).to receive(:touches_path?).with(source_path).and_return(diff_b_touches?)
+    end
+
+    context 'on absent diff filter' do
+      include_examples 'a method matcher'
+    end
+
+    context 'on single diff filter' do
+      let(:diffs) { [diff_a] }
+
+      context 'on not touching that diff' do
+        include_examples 'skipped candidate'
+      end
+
+      context 'on touching that diff' do
+        let(:diff_a_touches?) { true }
+
+        include_examples 'a method matcher'
+      end
+    end
+
+    context 'on multiple diff filter' do
+      let(:diffs) { [diff_a, diff_b] }
+
+      context 'on not touching any of the diffs' do
+        include_examples 'skipped candidate'
+      end
+
+      context 'on touching one diff' do
+        let(:diff_a_touches?) { true }
+
+        include_examples 'skipped candidate'
+      end
 
-    it 'returns expected subjects' do
-      expect(subject).to eql([])
+      context 'on touching both diffs' do
+        let(:diff_a_touches?) { true }
+        let(:diff_b_touches?) { true }
+
+        include_examples 'a method matcher'
+      end
     end
   end
 
@@ -73,10 +127,6 @@ def arguments
     end
 
     include_examples 'skipped candidate'
-
-    it 'returns expected subjects' do
-      expect(subject).to eql([])
-    end
   end
 
   context 'when method is defined without source location' do
@@ -90,10 +140,6 @@ def arguments
     end
 
     include_examples 'skipped candidate'
-
-    it 'returns expected subjects' do
-      expect(subject).to eql([])
-    end
   end
 
   context 'in module eval' do
@@ -106,10 +152,6 @@ def arguments
     end
 
     include_examples 'skipped candidate'
-
-    it 'returns expected subjects' do
-      expect(subject).to eql([])
-    end
   end
 
   context 'in class eval' do
@@ -122,10 +164,6 @@ def arguments
     end
 
     include_examples 'skipped candidate'
-
-    it 'returns expected subjects' do
-      expect(subject).to eql([])
-    end
   end
 
   context 'when method is defined once' do