diff --git a/Changelog.md b/Changelog.md index 670235ab1..4cf9edccd 100644 --- a/Changelog.md +++ b/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. diff --git a/lib/mutant/cli/command/environment.rb b/lib/mutant/cli/command/environment.rb index 096b41b26..581571e9c 100644 --- a/lib/mutant/cli/command/environment.rb +++ b/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 diff --git a/lib/mutant/matcher.rb b/lib/mutant/matcher.rb index 68fe1e978..7f58cce0f 100644 --- a/lib/mutant/matcher.rb +++ b/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? diff --git a/lib/mutant/matcher/config.rb b/lib/mutant/matcher/config.rb index e95b2c3f0..f6a04d555 100644 --- a/lib/mutant/matcher/config.rb +++ b/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)) diff --git a/lib/mutant/matcher/method.rb b/lib/mutant/matcher/method.rb index 05bb3141f..bbc54bf3b 100644 --- a/lib/mutant/matcher/method.rb +++ b/lib/mutant/matcher/method.rb @@ -120,6 +120,11 @@ 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) } @@ -127,6 +132,10 @@ def matched_view 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. diff --git a/lib/mutant/repository.rb b/lib/mutant/repository.rb index aca1e65aa..fd13dfc2e 100644 --- a/lib/mutant/repository.rb +++ b/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 diff --git a/spec/unit/mutant/cli_spec.rb b/spec/unit/mutant/cli_spec.rb index 5a140dd6c..a727ab646 100644 --- a/spec/unit/mutant/cli_spec.rb +++ b/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' diff --git a/spec/unit/mutant/matcher/config_spec.rb b/spec/unit/mutant/matcher/config_spec.rb index 56a0f1556..8966d42f3 100644 --- a/spec/unit/mutant/matcher/config_spec.rb +++ b/spec/unit/mutant/matcher/config_spec.rb @@ -13,8 +13,8 @@ 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 @@ -22,8 +22,8 @@ def apply 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('#') } end - - context 'with subject filter' do - let(:object) do - described_class::DEFAULT - .add(:subject_filters, 'foo') - end - - it { should eql('#') } - end end end diff --git a/spec/unit/mutant/matcher/method/instance_spec.rb b/spec/unit/mutant/matcher/method/instance_spec.rb index 212537ba5..79044410f 100644 --- a/spec/unit/mutant/matcher/method/instance_spec.rb +++ b/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 diff --git a/spec/unit/mutant/matcher_spec.rb b/spec/unit/mutant/matcher_spec.rb index ed153159b..ded903c43 100644 --- a/spec/unit/mutant/matcher_spec.rb +++ b/spec/unit/mutant/matcher_spec.rb @@ -7,11 +7,11 @@ def apply end let(:anon_matcher) { instance_double(Mutant::Matcher) } + let(:diffs) { [] } let(:env) { instance_double(Mutant::Env) } - let(:ignore_expressions) { [] } let(:expression_a) { expression('Foo::Bar#a', matcher_a) } let(:expression_b) { expression('Foo::Bar#b', matcher_b) } - let(:subject_filters) { [] } + let(:ignore_expressions) { [] } let(:matcher_a) do instance_double(Mutant::Matcher, call: [subject_a]) @@ -40,7 +40,7 @@ def apply ignore: ignore_expressions, subjects: [expression_a, expression_b], start_expressions: [], - subject_filters: subject_filters + diffs: diffs ) end @@ -49,7 +49,9 @@ def apply Mutant::Subject, 'subject a', expression: expression('Foo::Bar#a'), - inline_disabled?: false + inline_disabled?: false, + source_lines: 1..10, + source_path: 'subject/a.rb' ) end @@ -58,7 +60,9 @@ def apply Mutant::Subject, 'subject b', expression: expression('Foo::Bar#b'), - inline_disabled?: false + inline_disabled?: false, + source_lines: 100..101, + source_path: 'subject/b.rb' ) end @@ -98,25 +102,64 @@ def expression(input, matcher = anon_matcher) end end - context 'with subject filter' do - let(:subject_filters) do - [subject_a.public_method(:eql?)] - end + context 'with diffs' do + let(:diff_a) { instance_double(Mutant::Repository::Diff) } + let(:diff_b) { instance_double(Mutant::Repository::Diff) } - it 'returns expected subjects' do - expect(apply.call(env)).to eql([subject_a]) + let(:subject_a_touches_diff_a?) { false } + let(:subject_b_touches_diff_a?) { false } + let(:subject_a_touches_diff_b?) { false } + let(:subject_b_touches_diff_b?) { false } + + before do + allow(diff_a).to receive(:touches?) + .with(subject_a.source_path, subject_a.source_lines) + .and_return(subject_a_touches_diff_a?) + + allow(diff_a).to receive(:touches?) + .with(subject_b.source_path, subject_b.source_lines) + .and_return(subject_b_touches_diff_a?) + + allow(diff_b).to receive(:touches?) + .with(subject_a.source_path, subject_a.source_lines) + .and_return(subject_a_touches_diff_b?) + + allow(diff_b).to receive(:touches?) + .with(subject_b.source_path, subject_b.source_lines) + .and_return(subject_b_touches_diff_b?) end - end - context 'with subject ignore and filter' do - let(:ignore_expressions) { [subject_b.expression] } + context 'with one diff' do + let(:diffs) { [diff_a] } + + context 'when its touched by subject a' do + let(:subject_a_touches_diff_a?) { true } - let(:subject_filters) do - [subject_b.public_method(:eql?)] + it 'returns expected subjects' do + expect(apply.call(env)).to eql([subject_a]) + end + end end - it 'returns expected subjects' do - expect(apply.call(env)).to eql([]) + context 'with two diffs' do + let(:diffs) { [diff_a, diff_b] } + + context 'when one is touched by subject a' do + let(:subject_a_touches_diff_b?) { true } + + it 'returns expected subjects' do + expect(apply.call(env)).to eql([]) + end + end + + context 'when both are touched by subject a' do + let(:subject_a_touches_diff_a?) { true } + let(:subject_a_touches_diff_b?) { true } + + it 'returns expected subjects' do + expect(apply.call(env)).to eql([subject_a]) + end + end end end end diff --git a/spec/unit/mutant/repository/subject_filter_spec.rb b/spec/unit/mutant/repository/subject_filter_spec.rb deleted file mode 100644 index 29adbc08d..000000000 --- a/spec/unit/mutant/repository/subject_filter_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Mutant::Repository::SubjectFilter do - context '#call' do - subject { object.call(mutant_subject) } - - let(:object) { described_class.new(diff: diff) } - let(:diff) { instance_double(Mutant::Repository::Diff) } - let(:value) { instance_double(Object, 'value') } - - let(:mutant_subject) do - double( - 'Subject', - source_path: double('source path'), - source_lines: double('source lines') - ) - end - - before do - expect(diff).to receive(:touches?).with( - mutant_subject.source_path, - mutant_subject.source_lines - ).and_return(value) - end - - it 'connects return value to repository diff API' do - expect(subject).to be(value) - end - end -end