Skip to content

Commit

Permalink
Significantly optimize parsing overhead when --since is specified
Browse files Browse the repository at this point in the history
- Massively speeds up boot time on larger codebases and should be noticeable on all sizes. On `mutant` itself, when making a relatively targeted change (such as this one), boot time is reduced by about 0.7 seconds (~20% faster). On a larger project I work on, the speed up was 24 seconds or 2.5x faster overall! I left the original, full, subject filtering in place to simplify this change. This just short-circuits when the files we are interested in are not modified and `--since` is specified.
  * `mutant` run before on this change:
    ```
    Benchmark 1: bundle exec mutant run --zombie --since HEAD~1 --profile
    Time (mean ± σ):      4.192 s ±  0.350 s    [User: 8.567 s, System: 6.081 s]
    Range (min … max):    3.850 s …  4.960 s    10 runs
    ```
  * `mutant` run after on this change:
    ```
    Benchmark 1: bundle exec mutant run --zombie --since HEAD~1 --profile
    Time (mean ± σ):      3.535 s ±  0.213 s    [User: 6.890 s, System: 5.784 s]
    Range (min … max):    3.188 s …  3.784 s    10 runs
    ```
- Also reduces memory consumption significantly since the ASTs for all source files are never constructed, but I have not benchmarked the exact improvement.
  • Loading branch information
dgollahon committed May 14, 2023
1 parent e304940 commit eb7a079
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 58 deletions.
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -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 2-3x or greater boot time improvements. Memory consumption is also reduced.

* [#1378](https://github.com/mbj/mutant/pull/1379)

Add support for user defined mutation worker process hooks.
Expand Down
4 changes: 4 additions & 0 deletions lib/mutant/matcher/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ def merge(other)
)
end

def relevant_path?(source_path)
subject_filters.all? { |filter| filter.modified_path?(source_path) }
end

private

def present_attributes
Expand Down
5 changes: 5 additions & 0 deletions lib/mutant/matcher/method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 env.config.matcher.relevant_path?(source_path)

ast
.on_line(source_line)
.select { |view| view.node.type.eql?(self.class::MATCH_NODE_TYPE) && match?(view.node) }
Expand Down
4 changes: 4 additions & 0 deletions lib/mutant/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ def call(subject)
diff.touches?(subject.source_path, subject.source_lines)
end

def modified_path?(source_path)
diff.touches_path?(source_path)
end

end # SubjectFilter
end # Repository
end # Mutant
14 changes: 11 additions & 3 deletions lib/mutant/repository/diff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ class Error < RuntimeError; end
# @raise [RepositoryError]
# when git command failed
def touches?(path, line_range)
touched_paths
.from_right { |message| fail Error, message }
.fetch(path) { return false }
touched_path(path) { return false }
.touches?(line_range)
end

def touches_path?(path)
touched_path(path) { return false }

true
end

private

def repository_root
Expand All @@ -37,6 +41,10 @@ def repository_root
.fmap(&world.pathname.public_method(:new))
end

def touched_path(path, &block)
touched_paths.from_right { |message| fail Error, message }.fetch(path, &block)
end

def touched_paths
repository_root.bind(&method(:diff_index))
end
Expand Down
38 changes: 38 additions & 0 deletions spec/unit/mutant/matcher/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,42 @@ def apply
it { should eql('#<Mutant::Matcher::Config subject_filters: ["foo"]>') }
end
end

describe '#relevant_path?' do
subject { object.relevant_path?(path) }

let(:object) { described_class::DEFAULT.with(subject_filters: subject_filters) }

let(:path) { instance_double(Pathname) }
let(:subject_filters) { [] }
let(:passing_filter) do
instance_double(Mutant::Repository::SubjectFilter, modified_path?: true)
end
let(:failing_filter) do
instance_double(Mutant::Repository::SubjectFilter, modified_path?: false)
end

it 'accepts all paths when no subject filters are defined' do
expect(subject).to be(true)
end

context 'when a subject filter matches' do
let(:subject_filters) { [passing_filter] }

it 'is true' do
expect(subject).to be(true)
expect(passing_filter).to have_received(:modified_path?).with(path)
end
end

context 'when a subject filter does not match' do
let(:subject_filters) { [passing_filter, failing_filter] }

it 'is false' do
expect(subject).to be(false)
expect(passing_filter).to have_received(:modified_path?).with(path)
expect(passing_filter).to have_received(:modified_path?).with(path)
end
end
end
end
15 changes: 14 additions & 1 deletion spec/unit/mutant/matcher/method/instance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -21,7 +22,7 @@
let(:env) do
instance_double(
Mutant::Env,
config: Mutant::Config::DEFAULT,
config: config,
parser: Fixtures::TEST_ENV.parser,
world: world
)
Expand Down Expand Up @@ -58,6 +59,18 @@ def arguments
include_examples 'skipped candidate'
end

context 'when method is defined inside of a file removed by a subject filter' do
let(:scope) { base::WithMemoizer }
let(:expected_warnings) { [] }
let(:config) { super().with(matcher: instance_double(Mutant::Matcher::Config)) }

before do
expect(config.matcher).to receive(:relevant_path?).with(source_path).and_return(false) # TODO
end

include_examples 'skipped candidate'
end

context 'when method is defined inside eval' do
let(:scope) { base::WithMemoizer }
let(:method) { scope.instance_method(:boz) }
Expand Down
111 changes: 69 additions & 42 deletions spec/unit/mutant/repository/diff_spec.rb
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
# frozen_string_literal: true

describe Mutant::Repository::Diff do
describe '#touches?' do
def apply
subject.touches?(path, line_range)
end

subject { described_class.new(world: world, to: 'to_rev') }
subject { described_class.new(world: world, to: 'to_rev') }

let(:kernel) { class_double(Kernel) }
let(:line_range) { 4..5 }
let(:path) { Pathname.new('/foo/bar.rb') }
let(:pathname) { class_double(Pathname) }

let(:world) do
instance_double(
Mutant::World,
kernel: kernel,
pathname: pathname
)
end

let(:kernel) { class_double(Kernel) }
let(:line_range) { 4..5 }
let(:path) { Pathname.new('/foo/bar.rb') }
let(:pathname) { class_double(Pathname) }
let(:raw_expectations) do
[
{
receiver: world,
selector: :capture_stdout,
arguments: [%w[git rev-parse --show-toplevel]],
reaction: { return: Mutant::Either::Right.new("/foo\n") }
},
{
receiver: world,
selector: :capture_stdout,
arguments: [%w[git diff-index to_rev]],
reaction: { return: Mutant::Either::Right.new(index_stdout) }
},
*file_diff_expectations
]
end

let(:world) do
instance_double(
Mutant::World,
kernel: kernel,
pathname: pathname
)
end
let(:file_diff_expectations) { [] }

let(:allowed_paths) do
%w[/foo bar.rb baz.rb].to_h do |path|
[path, Pathname.new(path)]
end
let(:allowed_paths) do
%w[/foo bar.rb baz.rb].to_h do |path|
[path, Pathname.new(path)]
end
end

let(:file_diff_expectations) { [] }

let(:raw_expectations) do
[
{
receiver: world,
selector: :capture_stdout,
arguments: [%w[git rev-parse --show-toplevel]],
reaction: { return: Mutant::Either::Right.new("/foo\n") }
},
{
receiver: world,
selector: :capture_stdout,
arguments: [%w[git diff-index to_rev]],
reaction: { return: Mutant::Either::Right.new(index_stdout) }
},
*file_diff_expectations
]
end
before do
allow(pathname).to receive(:new, &allowed_paths.public_method(:fetch))
end

before do
allow(pathname).to receive(:new, &allowed_paths.public_method(:fetch))
describe '#touches?' do
def apply
subject.touches?(path, line_range)
end

context 'when file is not touched in the diff' do
Expand Down Expand Up @@ -123,4 +123,31 @@ def apply
end
end
end

describe '#touches_path?' do
def apply
subject.touches_path?(path)
end

context 'when file is not touched in the diff' do
let(:index_stdout) { '' }

it 'returns false' do
verify_events { expect(apply).to be(false) }
end
end

context 'when file is touched in the diff' do
let(:index_stdout) do
<<~STR
:000000 000000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 M\tbar.rb
:000000 000000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 M\tbaz.rb
STR
end

it 'returns true' do
verify_events { expect(apply).to be(true) }
end
end
end
end
36 changes: 24 additions & 12 deletions spec/unit/mutant/repository/subject_filter_spec.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# 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(: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

let(:mutant_subject) do
double(
'Subject',
source_path: double('source path'),
source_lines: double('source lines')
)
end
context '#call' do
subject { object.call(mutant_subject) }

before do
expect(diff).to receive(:touches?).with(
Expand All @@ -27,4 +27,16 @@
expect(subject).to be(value)
end
end

context '#modified_path?' do
subject { object.modified_path?(mutant_subject.source_path) }

before do
expect(diff).to receive(:touches_path?).with(mutant_subject.source_path).and_return(value)
end

it 'connects return value to repository diff API' do
expect(subject).to be(value)
end
end
end

0 comments on commit eb7a079

Please sign in to comment.