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

Parallel static analysis by default #10000

Merged
merged 1 commit into from Aug 12, 2021

Conversation

koic
Copy link
Member

@koic koic commented Aug 10, 2021

Modern PCs have multi-core. RuboCop has --parallel option, but it is not used by default. This means that there is extra core resources for static analysis because multi-core is not being used effectively.

This PR will be parallel processing by default, so RuboCop will run faster in proportion to number of cores. Options that can be parallel processing, such as --only, --except, and others have the
same default behavior.
However, there are options that cannot be parallel processing, such as auto-correction and others, so they are serial processing as before.

--parallel can be a hidden option for some users. This change is expected to reduce user's wait time for running RuboCop.

Below is an example on my development PC.

Before:

% cd path/to/rubocop
./exe/rubocop --display-time
(snip)

1293 files inspected, no offenses detected
Finished in 61.09493000002112 seconds

After:

% cd path/to/rubocop
./exe/rubocop --display-time
(snip)

1293 files inspected, no offenses detected
Finished in 10.13813699997263 seconds

If user's PC has a lot of cores, memory and RuboCop targets a lot of files to inspect, it will be effective.

One concern is that parallel processing uses more memory. For example, Ruby's MJIT is off by default as an example of concerning about memory consumption in a low memory production runtime (e.g. Heroku's minimum Dyno).
However, RuboCop is a development tool, I assume that it will be fine in most cases.

If user want to use it in a low memory production runtime, user can specify --no-parallel.

Note: JRuby and Windows are not supported based on #4537.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@dvandersluis
Copy link
Member

The failures on this and your other PRs are resolved by #9995 (and will be fixed by #9994), so it looks like you need to rebase.

@koic koic force-pushed the parallel_static_analysis_by_default branch from 35a9394 to 2514048 Compare August 11, 2021 03:58
Modern PCs have multi-core. RuboCop has `--parallel` option, but it is
not used by default. This means that there is extra core resources for
static analysis because multi-core is not being used effectively.

This PR will be parallel processing by default, so RuboCop will run
faster in proportion to number of cores. Options that can be
parallel processing, such as `--only`, `--except`, and others have the
same default behavior.
However, there are options that cannot be parallel processing, such as
auto-correction and others, so they are serial processing as before.

`--parallel` can be a hidden option for some users. This change is
expected to reduce user's wait time for running RuboCop.

Below is an example on my development PC.

Before:

```console
% cd path/to/rubocop
./exe/rubocop --display-time
(snip)

1293 files inspected, no offenses detected
Finished in 61.09493000002112 seconds
```

After:

```console
% cd path/to/rubocop
./exe/rubocop --display-time
(snip)

1293 files inspected, no offenses detected
Finished in 10.13813699997263 seconds
```

If user's PC has a lot of cores, memory and RuboCop targets a lot of files
to inspect, it will be effective.

One concern is that parallel processing uses more memory. For example,
Ruby's MJIT is off by default as an example of concerning about memory
consumption in a low memory production runtime (e.g. Heroku's minimum Dyno).
However, RuboCop is a development tool, I assume that it will be fine
in most cases.

If user want to use it in a low memory production runtime, user can
specify `--no-parallel`.

Note: JRuby and Windows are not supported based on rubocop#4537.
@koic koic force-pushed the parallel_static_analysis_by_default branch from 2514048 to 0739982 Compare August 11, 2021 04:31
@bbatsov bbatsov merged commit 0114ebc into rubocop:master Aug 12, 2021
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 12, 2021

I think that's a good move! The main reason why we didn't do this earlier is that for a while the parallel analysis was rough around the edges, but given the massive time savings, I think it makes for a a better default.

P.S. 10000 is the quite the magic number! 🚀 🏅

@koic koic deleted the parallel_static_analysis_by_default branch August 12, 2021 05:17
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Aug 13, 2021
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.

None yet

3 participants