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. In my larger project on very small single subject change it reduced memory allocations by 69MB (80% reduction) and reduced "retained" memory by 25MB (99.4% reduction).
  • Loading branch information
dgollahon committed May 14, 2023
1 parent e304940 commit b25306a
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 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.
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 b25306a

Please sign in to comment.