Rubocop: Quick performance review

[marcandre]

I had fun checking what takes time when inspecting RuboCop's own files.

Once I excluded the worst offender (see #8021), here's the top 20:

     584  (    3.7%)  RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets#on_array
     425  (    2.7%)  RuboCop::Cop::Style::RedundantSelf#on_block
     390  (    2.5%)  RuboCop::Cop::Layout::SpaceBeforeFirstArg#on_send
     356  (    2.3%)  RuboCop::RSpec::TopLevelDescribe#on_send
     289  (    1.8%)  RuboCop::Cop::RSpec::SubjectStub#on_block
     255  (    1.6%)  RuboCop::Cop::Layout::IndentationConsistency#on_begin
     249  (    1.6%)  RuboCop::Cop::Layout::SpaceInsideReferenceBrackets#on_send
     246  (    1.6%)  RuboCop::Cop::Layout::FirstArgumentIndentation#on_send
     236  (    1.5%)  RuboCop::Cop::RSpec::FactoryBot::AttributeDefinedStatically#on_block
     206  (    1.3%)  RuboCop::Cop::Layout::DefEndAlignment#on_send
     184  (    1.2%)  RuboCop::Cop::Layout::SpaceInsideHashLiteralBraces#on_hash
     183  (    1.2%)  RuboCop::Cop::Style::TrailingMethodEndStatement#on_def
     176  (    1.1%)  RuboCop::Cop::Performance::FixedSize#on_send
     160  (    1.0%)  RuboCop::Cop::StringHelp#on_str
     154  (    1.0%)  RuboCop::Cop::RSpec::ReturnFromStub#on_block
     153  (    1.0%)  RuboCop::Cop::RSpec::InstanceVariable#on_block
     152  (    1.0%)  RuboCop::Cop::Interpolation#on_dstr
     143  (    0.9%)  RuboCop::Cop::RSpec::LetSetup#on_block
     138  (    0.9%)  RuboCop::Cop::RSpec::MultipleSubjects#on_block

The fastest cops take 0.0%, so there's potential for improvement.

First comes RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets. It uses tokens which should probably be optimized (e.g. using bsearch_index), but I'm not sure it should even be looking at tokens at all.

Second comes RuboCop::Cop::Style::RedundantSelf. Much more involved because it keeps tracks of scopes, but it should probably use VariableForce, no?

The fact that RuboCop::Cop::RSpec::FactoryBot::AttributeDefinedStatically is in the top 20 when the whole source code of Rubocop doesn't use FactoryBot is alarming...

I'll create a PR to add some profiling capacity

[bbatsov]

Great initiative! I have to admit that lately we haven't paid as much attention to performance as we should have. Looking at the list of offenders it seems a lot of them are space-related (and probably implemented in a similar fashion).

[andrykonchin]

I've run Rubocop with profiler (stackprof) on a large Rails application (proprietary) and received a little bit different top 10 list:

CLICK ME

stackprof tmp/stackprof-cpu.dump --text --sort-total --limit 150 | grep RuboCop::Cop
      1772   (6.8%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideReferenceBrackets#on_send
       971   (3.7%)           0   (0.0%)     RuboCop::Cop::Style::TrailingMethodEndStatement#trailing_end?
       971   (3.7%)           0   (0.0%)     RuboCop::Cop::Style::TrailingMethodEndStatement#on_def
       969   (3.7%)           0   (0.0%)     RuboCop::Cop::Style::TrailingMethodEndStatement#body_and_end_on_same_line?
       966   (3.7%)           2   (0.0%)     RuboCop::Cop::Style::TrailingMethodEndStatement#end_token
       374   (1.4%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets#on_array
       357   (1.4%)           1   (0.0%)     RuboCop::Cop::Layout::IndentationWidth#check_indentation
       353   (1.3%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets#array_brackets
       352   (1.3%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets#left_array_bracket
       327   (1.2%)           0   (0.0%)     RuboCop::Cop::Layout::IndentationWidth#check_members
       315   (1.2%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideHashLiteralBraces#hash_literal_with_braces
       315   (1.2%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceInsideHashLiteralBraces#on_hash
       294   (1.1%)           0   (0.0%)     RuboCop::Cop::RangeHelp#column_offset_between
       294   (1.1%)         281   (1.1%)     RuboCop::Cop::RangeHelp#effective_column
       294   (1.1%)         140   (0.5%)     RuboCop::Cop::Style::RedundantSelf#add_scope
       278   (1.1%)           0   (0.0%)     RuboCop::Cop::Layout::IndentationWidth#on_class
       277   (1.1%)           0   (0.0%)     RuboCop::Cop::Layout::IndentationWidth#check_members_for_normal_style
       251   (1.0%)           2   (0.0%)     RuboCop::Cop::Layout::EmptyLines#investigate
       247   (0.9%)           0   (0.0%)     RuboCop::Cop::Layout::EndAlignment#check_other_alignment
       242   (0.9%)         194   (0.7%)     RuboCop::Cop::Badge#to_s
       235   (0.9%)           7   (0.0%)     RuboCop::Cop::Style::NumericPredicate#on_send
       231   (0.9%)           0   (0.0%)     RuboCop::Cop::Layout::IndentationConsistency#check
       229   (0.9%)           0   (0.0%)     RuboCop::Cop::Layout::IndentationConsistency#on_begin
       219   (0.8%)           1   (0.0%)     RuboCop::Cop::Layout::IndentationConsistency#check_normal_style
       216   (0.8%)           0   (0.0%)     RuboCop::Cop::Layout::SpaceBeforeFirstArg#on_send
       207   (0.8%)           0   (0.0%)     RuboCop::Cop::RangeHelp#column_offset_between

So it's definitely makes sense to have some performance testing methodology - e.g. to choose some large public Ruby codebases and measure performance improvements against them. I wouldn't say the Rubocop source code is either typical or large.

[marcandre]

Super interesting @andrykonchin

The listing I gave was with --method 'RuboCop::Cop::Commissioner#trigger_responding_cops', to check from the main dispatch point for slow cops.

If the O notation of some cops is wrong (i.e. not O(n)) then a codebase of longer/shorter files will also show a different portrait.

Still, many cops are showing in both reports...

[andrykonchin]

The listing I gave was with --method 'RuboCop::Cop::Commissioner#trigger_responding_cops', to check from the main dispatch point for slow cops.

Thank you. It's very helpful.

I profiled rubocop on the whole my project (earlier I ran it only on a subdirectory app/) and again have very different results with filtering by the #trigger_responding_cops method. And but looks like there are obvious bottlenecks:

    177495  (   44.2%)  RuboCop::Cop::Style::NumericLiterals#on_int
    128013  (   31.9%)  RuboCop::Cop::Layout::SpaceInsideArrayLiteralBrackets#on_array
    25870  (    6.4%)  RuboCop::Cop::StringHelp#on_str
    17734  (    4.4%)  RuboCop::Cop::Layout::SpaceInsideReferenceBrackets#on_send
    2423  (    0.6%)  RuboCop::Cop::Layout::SpaceInsideHashLiteralBraces#on_hash
    2358  (    0.6%)  RuboCop::Cop::Style::WordArray#on_array

I'm exited to play with performance issues and testing so going to choose several Ruby/Rails projects and analyze them with stackprof.

I am wondering whether it makes sense to use another sampling profiler or even tracing one?

[tas50]

If you're looking for a good size non-Rails ruby project to test rubocop with: https://github.com/chef/chef

[andrykonchin]

Just for the record. I've profiled rubocop-rspec cops and fixed most not efficient ones:

• RSpec/SubjectStub
• RSpec/ReturnFromStub
• RSpec/InstanceVariable
• FactoryBot/AttributeDefinedStatically
• RSpec/NestedGroups
• RSpec/LetSetup

https://github.com/rubocop-hq/rubocop-rspec/pulls?q=is%3Apr+author%3Aandrykonchin+is%3Aclosed+performance+

[marcandre]

Great!

I was wondering about the best way to be able to do profiling. I'm not super happy about https://github.com/rubocop-hq/rubocop/pull/8242, comments welcome.

@andrykonchin were some of these cops using tokens?

I was looking at SpaceAroundMethodCallOperator and it's quite spectacular, it does a search in all the tokens 3 times in a row 🙄. This is despite including SurroundingSpace which builds an index table for the tokens. I wish I was familiar enough with these cops to know if the tokens are really the way to go about this in the first place.

[andrykonchin]

@andrykonchin were some of these cops using tokens?

No. The common issue is excessive recursion and usage of def_node_search macros.

I was looking at SpaceAroundMethodCallOperator and it's quite spectacular

Yeah, micro optimizations will not help much here. These inefficient cops should be rethought and rewritten. Hope I will be able to try after a short break.

[tleish]

I've run Rubocop with profiler (stackprof) on a large Rails application (proprietary) and received a little bit different top 10 list:

Does profiling against a large project make sense? Is the output reporting how long a single cop took when running one-time, or do cops bubble to the top because they are fully executed more than other cops?

[marcandre]

I've run Rubocop with profiler (stackprof) on a large Rails application (proprietary) and received a little bit different top 10 list:

Does profiling against a large project make sense? Is the output reporting how long a single cop took when running one-time, or do cops bubble to the top because they are fully executed more than other cops?

Yes, but a small project works too. The latter.

We have some builtin tools now btw.

[tleish]

@marcandre - is there a standard "small project" that you've been using for profiling?

[marcandre]

I've been using rubocop itself, or a subset of cops.