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

Improve performance when user does not override default RSpec Pattern config #507

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

walf443
Copy link
Contributor

@walf443 walf443 commented Nov 26, 2017

I noticed that RuboCop::Cop::RSpec::Cop#rspec_pattern is not so fast.
RSpec Pattern setting is not defined in most case, I think.
So change to cache building Regexp for default case.

Here is the stackprof result of running ./bin/rubocop at rubocop repositories on master

bundle exec stackprof tmp/stackprof-cpu-rubocop.dump
==================================
  Mode: cpu(1000)
  Samples: 52150 (0.02% miss rate)
  GC: 3229 (6.19%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      8039  (15.4%)        4264   (8.2%)     RuboCop::AST::Node#each_child_node
      3352   (6.4%)        3352   (6.4%)     block (2 levels) in <class:Node>
      3229   (6.2%)        3229   (6.2%)     (garbage collection)
      5362  (10.3%)        2846   (5.5%)     Parser::Lexer#advance
      2386   (4.6%)        2386   (4.6%)     AST::Node#to_a
      2133   (4.1%)        2133   (4.1%)     Parser::Source::Buffer#slice
      1421   (2.7%)        1061   (2.0%)     Parser::Source::Buffer#line_for_position
      1555   (3.0%)         908   (1.7%)     RuboCop::Cop::Cop#cop_config
       570   (1.1%)         570   (1.1%)     #<Module:0x00007f9dddefffb0>.of
       557   (1.1%)         557   (1.1%)     Parser::Source::Range#initialize
       620   (1.2%)         549   (1.1%)     RuboCop::Cop::RSpec::Cop#rspec_pattern
       651   (1.2%)         538   (1.0%)     RuboCop::ConfigStore#for
       479   (0.9%)         479   (0.9%)     RuboCop::Formatter::ProgressFormatter#report_file_as_mark
      1001   (1.9%)         440   (0.8%)     AST::Node#initialize
       592   (1.1%)         399   (0.8%)     RuboCop::Cop::Cop#cop_name
       429   (0.8%)         398   (0.8%)     RuboCop::Cop::VariableForce#scanned_node?
       561   (1.1%)         397   (0.8%)     Parser::AST::Node#assign_properties
       731   (1.4%)         396   (0.8%)     Parser::Source::Buffer#line_for
     18229  (35.0%)         360   (0.7%)     RuboCop::Cop::Commissioner#on_send
       340   (0.7%)         340   (0.7%)     Parser::Source::Buffer#line_begins
       709   (1.4%)         338   (0.6%)     Parser::Source::Buffer#column_for_position
       333   (0.6%)         333   (0.6%)     AST::Node#eql?
       332   (0.6%)         332   (0.6%)     Parser::Lexer::Literal#coerce_encoding
       324   (0.6%)         324   (0.6%)     RuboCop::Cop::Cop#initialize
       322   (0.6%)         322   (0.6%)     RuboCop::Cop::Cop.badge
       310   (0.6%)         310   (0.6%)     RuboCop::AST::Node#parent
       308   (0.6%)         308   (0.6%)     RuboCop::Cop::IgnoredNode#ignored_nodes
      1009   (1.9%)         304   (0.6%)     RuboCop::Config#for_cop
       282   (0.5%)         282   (0.5%)     RuboCop::AST::MethodDispatchNode#setter_method?
      2342   (4.5%)         282   (0.5%)     RuboCop::Cop::Commissioner#remove_irrelevant_cops

Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes.
  • The build (bundle exec rake) is passing.

If you have created a new cop:

  • Added the new cop to config/default.yml.
  • The cop includes examples of good and bad code.
  • You have tests for both code that should be reported and code that is good.

@@ -58,7 +65,17 @@ def relevant_rubocop_rspec_file?(file)
end

def rspec_pattern
Regexp.union(rspec_pattern_config.map(&Regexp.public_method(:new)))
if rspec_config_defined?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently the only RSpec configuration is Patterns, but that may not be the case in the future. Perhaps it would be better to check for rspec_pattern_config_defined? – e.g.

def rspec_pattern_config_defined?
  rspec_config_defined? &&
    config
      .for_all_cops
      .fetch('RSpec')
      .key?('Patterns')
end

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I'll change to check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bquorning updated

@bquorning bquorning merged commit d270799 into rubocop:master Nov 29, 2017
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

2 participants