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

[Fix #10764] Fix performance issue for Layout/FirstHashElementIndentation and Layout/FirstArrayElementIndentation #10777

Conversation

j-miyake
Copy link
Contributor

@j-miyake j-miyake commented Jun 30, 2022

Fixes #10764.

In rubocop v1.31.0, MultilineElementIndentation that is used by Layout/FirstHashElementIndentation and Layout/FirstArrayElementIndentation walks all nodes through in the code to find a hash pair node where its value part begins with the given left-brace; That computation is expensive, so I made a change so that the hash pair is passed by each class that uses MultilineElementIndentation.

I ran rubocop by the following command to make sure the performance problem was fixed by this PR. The input file is the file provided in #10764 (comment)

$ time exe/rubocop --cache false test.rb

Result:

  • With v1.30.1
exe/rubocop --cache false test.rb  1.51s user 0.33s system 41% cpu 4.410 total
  • With v1.31.0
exe/rubocop --cache false test.rb  3.26s user 0.29s system 84% cpu 4.199 total
  • With this PR
exe/rubocop --cache false test.rb  1.55s user 0.32s system 62% cpu 3.012 total

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.

@j-miyake j-miyake force-pushed the fix_performance_issue_of_multiline_element_indentation branch 2 times, most recently from 99cefa8 to 73378f2 Compare June 30, 2022 11:22
…Indentation and Layout/FirstArrayElementIndentation

Fixes rubocop#10764.

In rubocop v1.31.0, `MultilineElementIndentation` that is used by `Layout/FirstHashElementIndentation` and `Layout/FirstArrayElementIndentation` walks all nodes through in the code to find a hash pair node where its value part begins with the given left-brace; That computation is expensive, so I made a change so that the hash pair is passed by each class that includes `MultilineElementIndentation`.
@j-miyake j-miyake force-pushed the fix_performance_issue_of_multiline_element_indentation branch from 73378f2 to d5c49f9 Compare June 30, 2022 11:39
@j-miyake j-miyake marked this pull request as ready for review July 1, 2022 01:45
@bbatsov bbatsov merged commit b8a1cb0 into rubocop:master Jul 7, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 7, 2022

Looks good. Thanks!

@j-miyake j-miyake deleted the fix_performance_issue_of_multiline_element_indentation branch July 7, 2022 08:34
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.

Performance issues with 1.31.0 (Layout/FirstHashElementIndentation and Layout/FirstArrayElementIndentation)
2 participants