Quick performance review #8022

marcandre · 2020-05-24T08:25:52Z

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

The text was updated successfully, but these errors were encountered:

bbatsov · 2020-05-24T11:50:48Z

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 · 2020-05-27T23:32:46Z

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 · 2020-05-28T00:22:20Z

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 · 2020-05-29T00:07:08Z

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 · 2020-05-29T03:13:43Z

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

andrykonchin · 2020-07-05T13:44:42Z

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 · 2020-07-05T14:38:05Z

Great!

I was wondering about the best way to be able to do profiling. I'm not super happy about #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 · 2020-07-05T18:14:30Z

@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 · 2020-09-29T09:32:29Z

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 · 2020-09-29T16:09:07Z

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 · 2020-09-29T16:38:16Z

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

marcandre · 2020-09-29T17:24:57Z

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

marcandre added enhancement good first issue Easy task, suitable for newcomers to the project help wanted refactoring labels May 24, 2020

rrosenblum mentioned this issue May 27, 2020

Minor performance tweaks #8048

Closed

This was referenced Jun 6, 2020

[WIP] Minor optimizations #8105

Closed

RSpec/SubjectStub. Refactor and decrease complexity rubocop/rubocop-rspec#925

Merged

pirj mentioned this issue Jun 14, 2020

Deduplicate logic in ExampleGroup rubocop/rubocop-rspec#935

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quick performance review #8022

Quick performance review #8022

marcandre commented May 24, 2020

bbatsov commented May 24, 2020

andrykonchin commented May 27, 2020

marcandre commented May 28, 2020

andrykonchin commented May 29, 2020 •

edited

tas50 commented May 29, 2020

andrykonchin commented Jul 5, 2020 •

edited

marcandre commented Jul 5, 2020

andrykonchin commented Jul 5, 2020

tleish commented Sep 29, 2020

marcandre commented Sep 29, 2020 •

edited

tleish commented Sep 29, 2020

marcandre commented Sep 29, 2020

Quick performance review #8022

Quick performance review #8022

Comments

marcandre commented May 24, 2020

bbatsov commented May 24, 2020

andrykonchin commented May 27, 2020

marcandre commented May 28, 2020

andrykonchin commented May 29, 2020 • edited

tas50 commented May 29, 2020

andrykonchin commented Jul 5, 2020 • edited

marcandre commented Jul 5, 2020

andrykonchin commented Jul 5, 2020

tleish commented Sep 29, 2020

marcandre commented Sep 29, 2020 • edited

tleish commented Sep 29, 2020

marcandre commented Sep 29, 2020

andrykonchin commented May 29, 2020 •

edited

andrykonchin commented Jul 5, 2020 •

edited

marcandre commented Sep 29, 2020 •

edited