Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Significantly optimize parsing overhead when --since is specified #1382

Merged
merged 2 commits into from
May 22, 2023

Conversation

dgollahon
Copy link
Collaborator

@dgollahon dgollahon commented May 14, 2023

  • 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).
  • Also removes some redundant skipped candidate specs

@dgollahon dgollahon changed the title Significantly optimize parsing overhead when --since is specified Significantly optimize parsing overhead when --since is specified May 14, 2023
@@ -120,6 +120,11 @@ def subject_config(node)
def matched_view
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbj there is something wrong or incomplete with mutant or the zombification process. When I tried to mutate Mutant::Matcher::Method it found zero subjects. I had the same behavior on master as well so it was not introduced by this change.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly for me on main. Can you share a reproducing CLI?

@dgollahon dgollahon force-pushed the optimize-parsing branch 4 times, most recently from bc2dc3f to b25306a Compare May 14, 2023 19:32
lib/mutant/matcher/config.rb Outdated Show resolved Hide resolved
lib/mutant/repository/diff.rb Outdated Show resolved Hide resolved
- An identical expectation is included via the `skipped candidate` shared example in `spec/shared/method_matcher_behavior.rb`.
@dgollahon dgollahon force-pushed the optimize-parsing branch 3 times, most recently from 3672729 to ad93a3f Compare May 21, 2023 20:18
…iffs

* Avoids parsing source files that live outside the specified diff.
* 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).
@@ -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,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgollahon I could get a rid of the entire subject_filters after checking private integrations, they where ported to the hooks subsystem so no need to maintain this API. this overall makes this commit much nicer than previous versions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thank you for validating and simplifying that. I think it's much nicer code-wise, just didn't realize it was an option.

@mbj mbj self-requested a review May 22, 2023 00:41
@mbj mbj merged commit d18fb50 into main May 22, 2023
@mbj mbj deleted the optimize-parsing branch May 22, 2023 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants