8000 Faster runs via RuboCop::Runner by jdelStrother · Pull Request #573 · sds/haml-lint · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Faster runs via RuboCop::Runner #573

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

Merged
merged 4 commits into from
Jun 24, 2025
Merged

Faster runs via RuboCop::Runner #573

merged 4 commits into from
Jun 24, 2025

Conversation

jdelStrother
Copy link
Contributor
@jdelStrother jdelStrother commented Jun 22, 2025

Every time we call RuboCop::CLI#run (which happens once per file being linted), it sets up a new RuboCop::Runner.
This is pretty slow, especially from RuboCop::Options#parse and RuboCop::Runner#mobilized_cop_classes.

We can skip all this repeated setup by using RuboCop::Runner directly. This also means we don't have to capture the stdout/stderr streams from CLI, since the relevant information is available directly on the runner instance.

Before:

626 files inspected, 0 lints detected

________________________________________________________
Executed in   23.22 secs    fish           external
   usr time   20.12 secs    0.32 millis   20.12 secs
   sys time    2.57 secs    2.49 millis    2.56 secs

After:

626 files inspected, 0 lints detected

________________________________________________________
Executed in   17.64 secs    fish           external
   usr time   15.30 secs    0.34 millis   15.30 secs
   sys time    1.93 secs    2.60 millis    1.93 secs

I've introduced a subclass of RuboCop::Runner to handle some of the customizations we need. ruby-lsp have a similar approach here - https://github.com/Shopify/ruby-lsp/blob/13bb8b0addf96248a951dca988e40127c86b6ba8/lib/ruby_lsp/requests/support/rubocop_runner.rb#L54, which might be useful for comparison.

I'd moved this to a class-level instance variable back in bd82186, but only to avoid issues with Marshal.dump on the linter instance. Instead, let's override marshal_dump to exclude the problematic ivars
Every time we call RuboCop::CLI#run (which happens once per file being linted), it sets up a new RuboCop::Runner.
This is pretty slow, especially from RuboCop::Options#parse and RuboCop::Runner#mobilized_cop_classes.

We can skip all this repeated setup by using RuboCop::Runner directly. This also means we don't have to capture the stdout/stderr streams from CLI, since the relevant information is available directly on the runner instance.
If we're not using RuboCop::CLI, I think we may as well collect the offenses in the runner directly
@jdelStrother
Copy link
Contributor Author

I've also got a POC here that makes use of RuboCop's cache, which dramatically improves times on already-processed files:

626 files inspected, 0 lints detected

________________________________________________________
Executed in    5.22 secs    fish           external
   usr time    4.30 secs    0.32 millis    4.30 secs
   sys time    0.88 secs    4.40 millis    0.87 secs

RuboCop doesn't use its cache on code from stdin, so when we generate ruby code from the haml file for RuboCop to lint, we could write it to, eg, tmp/haml-lint/app/views/posts/index.html.haml.rb.
Relying on a tmp directory doesn't seem great though, so maybe we can defer that idea to a future PR.

@sds sds merged commit 8b94737 into sds:main Jun 24, 2025
34 of 35 checks passed
@sds
Copy link
Owner
sds commented Jun 24, 2025

Amazing work! Thank you

@jdelStrother jdelStrother deleted the rubocop-runner branch June 27, 2025 15:46
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
0