Performance. RSpec/InstanceVariable

[andrykonchin]

Optimized performance of RSpec/InstanceVariable cop.

Changes

• use RuboCop::RSpec::TopLevelGroup to process only top level groups
• added spec to document existing behavior for multiple nested example groups

The cop used #on_block callback and processed nested context/describe blocks multiple times. Now it uses RuboCop::RSpec::TopLevelGroup module and declares #on_top_level_group callback so nested groups aren't processed

Performance measurements

Timing is changed from 5.6% (InstanceVariable#on_block) to 2.6% (RuboCop::RSpec::TopLevelGroup#on_block)

Before

stackprof tmp/stackprof-cpu-gitlab.master.with-rubocop-rspec.InstanceVariable.dump --method 'RuboCop::Cop::RSpec::InstanceVariable#on_block'
RuboCop::Cop::RSpec::InstanceVariable#on_block (/Users/andrykonchin/projects/rubocop-rspec/lib/rubocop/cop/rspec/instance_variable.rb:72)
  samples:     0 self (0.0%)  /   1783 total (5.6%)
  callers:
    1783  (  100.0%)  RuboCop::Cop::Commissioner#trigger_responding_cops
      80  (    4.5%)  RuboCop::Cop::RSpec::InstanceVariable#ivar_usage
  callees (1783 total):
    1695  (   95.1%)  RuboCop::Cop::RSpec::InstanceVariable#ivar_usage
      88  (    4.9%)  RuboCop::Cop::RSpec::InstanceVariable#spec_group?
      62  (    3.5%)  RuboCop::Cop::Cop#add_offense
      17  (    1.0%)  RuboCop::Cop::RSpec::InstanceVariable#valid_usage?
       1  (    0.1%)  RuboCop::Cop::RSpec::InstanceVariable#assignment_only?
  code:
                                  |    72  |         def on_block(node)
   88    (0.3%)                   |    73  |           return unless spec_group?(node)
                                  |    74  |
 1695    (5.4%)                   |    75  |           ivar_usage(node) do |ivar, name|
   17    (0.1%)                   |    76  |             next if valid_usage?(ivar)
    1    (0.0%)                   |    77  |             next if assignment_only? && !ivar_assigned?(node, name)
                                  |    78  |
   62    (0.2%)                   |    79  |             add_offense(ivar)
                                  |    80  |           end

After

stackprof tmp/stackprof-cpu-gitlab.master.with-rubocop-rspec.InstanceVariable.3.dump --method 'RuboCop::RSpec::TopLevelGroup#on_block'
RuboCop::RSpec::TopLevelGroup#on_block (/Users/andrykonchin/projects/rubocop-rspec/lib/rubocop/rspec/top_level_group.rb:13)
  samples:    16 self (0.1%)  /    764 total (2.6%)
  callers:
     764  (  100.0%)  RuboCop::Cop::Commissioner#trigger_responding_cops
  callees (748 total):
     664  (   88.8%)  RuboCop::Cop::RSpec::InstanceVariable#on_top_level_group
      84  (   11.2%)  RuboCop::RSpec::TopLevelGroup#top_level_group?
  code:
                                  |    13  |       def on_block(node)
   16    (0.1%) /    16   (0.1%)  |    14  |         return unless respond_to?(:on_top_level_group)
   84    (0.3%)                   |    15  |         return unless top_level_group?(node)
                                  |    16  |
  664    (2.3%)                   |    17  |         on_top_level_group(node)
                                  |    18  |       end

Measurements approach

Used stackprof profiler to measure proportion of the cop timing. Running Rubocop on the GitLab project specs.

Run only one cope without caching and skip config with command

bundle exec exe/rubocop --cache false --out gitlab-specs.out --force-default-config  --require rubocop-rspec --only RSpec/InstanceVariable ../rubocop-profiling-examples/gitlabhq/spec

---

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.
• Updated documentation.
• Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

[pirj]

Awesome!

Please ignore CodeClimate, as it's not a required build, and you've actually reduced the perceived complexity of that method by getting rid of a guard statement.