Skip to content

List/LastKeywordArgument: Delay computing file path

Peter Leitzen requested to merge pl-rubocop-last-keyword-argument-speed-up into master

What does this MR do and why?

This MR improves speed of 👮 rule List/LastKeywordArgument by delaying the computation of file path.

It also makes sure that this cop is never run in Ruby 3.0 because it's useless because deprecation warnings turn into errors anyway.

Screenshots or screen recordings

Stack Profile

Run bin/rubocop-profile spec/models/project*

Before

==================================
  Mode: wall(1000)
  Samples: 9113 (1.87% miss rate)
  GC: 628 (6.89%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      1034  (11.3%)         646   (7.1%)     RuboCop::AST::Descendence#each_child_node
       461   (5.1%)         461   (5.1%)     block (2 levels) in <class:Node>
       327   (3.6%)         327   (3.6%)     (marking)
       298   (3.3%)         298   (3.3%)     (sweeping)
       502   (5.5%)         298   (3.3%)     Parser::Lexer#advance
       171   (1.9%)         171   (1.9%)     RuboCop::AST::Node#node_parts
       156   (1.7%)         156   (1.7%)     Parser::Source::Buffer#slice
       203   (2.2%)         155   (1.7%)     Parser::Source::Buffer#line_index_for_position
       116   (1.3%)         116   (1.3%)     RuboCop::AST::Node#parent
      4920  (54.0%)         110   (1.2%)     RuboCop::Cop::Commissioner#trigger_responding_cops
       126   (1.4%)         107   (1.2%)     AST::Node#initialize
       546   (6.0%)         103   (1.1%)     <top (required)>
        98   (1.1%)          98   (1.1%)     Psych::TreeBuilder#event_location
        87   (1.0%)          87   (1.0%)     RuboCop::AST::SendNode#first_argument_index
        73   (0.8%)          73   (0.8%)     RuboCop::Cop::Base.inherited
        73   (0.8%)          73   (0.8%)     Parser::Source::Range#initialize
        65   (0.7%)          65   (0.7%)     Psych::Nodes::Scalar#initialize
        65   (0.7%)          62   (0.7%)     RuboCop::AST::NodePattern::MethodDefiner#def_helper
        67   (0.7%)          62   (0.7%)     RuboCop::AST::ProcessedSource#sorted_tokens
        62   (0.7%)          59   (0.6%)     RuboCop::Cop::Lint::LastKeywordArgument#known_match?
        58   (0.6%)          58   (0.6%)     Gitlab::Styles::Rubocop::ModelHelpers#in_model?
        56   (0.6%)          56   (0.6%)     Gitlab::Styles::Rubocop::ModelHelpers#in_model?
        62   (0.7%)          54   (0.6%)     RuboCop::Config#make_excludes_absolute
        52   (0.6%)          52   (0.6%)     Gitlab::Styles::Rubocop::ModelHelpers#in_model?
        57   (0.6%)          52   (0.6%)     RuboCop::RSpec::Language#rspec?
        72   (0.8%)          44   (0.5%)     <module:Parser>
        45   (0.5%)          44   (0.5%)     RuboCop::Cop::Base.badge
        54   (0.6%)          43   (0.5%)     RuboCop::AST::NodePattern::LexerRex#next_token
       459   (5.0%)          42   (0.5%)     RuboCop::AST::Descendence#visit_descendants
        40   (0.4%)          40   (0.4%)     RuboCop::QAHelpers#in_qa_file?

Note RuboCop::Cop::Lint::LastKeywordArgument#known_match? in top 30.

After

==================================
  Mode: wall(1000)
  Samples: 9624 (2.12% miss rate)
  GC: 639 (6.64%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      1106  (11.5%)         674   (7.0%)     RuboCop::AST::Descendence#each_child_node
       484   (5.0%)         484   (5.0%)     block (2 levels) in <class:Node>
       335   (3.5%)         335   (3.5%)     (marking)
       301   (3.1%)         301   (3.1%)     (sweeping)
       482   (5.0%)         281   (2.9%)     Parser::Lexer#advance
       197   (2.0%)         197   (2.0%)     RuboCop::AST::Node#node_parts
       195   (2.0%)         195   (2.0%)     Parser::Source::Buffer#slice
       193   (2.0%)         134   (1.4%)     Parser::Source::Buffer#line_index_for_position
       154   (1.6%)         124   (1.3%)     AST::Node#initialize
       115   (1.2%)         115   (1.2%)     RuboCop::AST::Node#parent
      5201  (54.0%)         112   (1.2%)     RuboCop::Cop::Commissioner#trigger_responding_cops
       582   (6.0%)         109   (1.1%)     <top (required)>
       105   (1.1%)         105   (1.1%)     Psych::TreeBuilder#event_location
        97   (1.0%)          97   (1.0%)     RuboCop::AST::SendNode#first_argument_index
        90   (0.9%)          90   (0.9%)     Gitlab::Styles::Rubocop::ModelHelpers#in_model?
        75   (0.8%)          75   (0.8%)     Parser::Source::Range#initialize
        82   (0.9%)          72   (0.7%)     RuboCop::AST::NodePattern::MethodDefiner#def_helper
        65   (0.7%)          64   (0.7%)     RuboCop::RSpec::Language#rspec?
        62   (0.6%)          62   (0.6%)     Gitlab::Styles::Rubocop::ModelHelpers#in_model?
        68   (0.7%)          59   (0.6%)     RuboCop::AST::ProcessedSource#sorted_tokens
        55   (0.6%)          55   (0.6%)     RuboCop::Cop::Base.inherited
        55   (0.6%)          55   (0.6%)     Psych::Nodes::Scalar#initialize
        63   (0.7%)          55   (0.6%)     RuboCop::Config#make_excludes_absolute
       357   (3.7%)          54   (0.6%)     RuboCop::AST::Descendence#each_node
        61   (0.6%)          52   (0.5%)     RuboCop::AST::NodePattern::LexerRex#next_token
        50   (0.5%)          49   (0.5%)     RuboCop::Cop::Base.badge
       508   (5.3%)          49   (0.5%)     RuboCop::AST::Descendence#visit_descendants
        48   (0.5%)          48   (0.5%)     RuboCop::QAHelpers#in_qa_file?
        77   (0.8%)          45   (0.5%)     <module:Parser>
        43   (0.4%)          43   (0.4%)     Psych::ClassLoader#load

How to set up and validate locally

  1. With Ruby 2.7, run rubocop --show-cops Lint/LastKeywordArgument
  2. Verify that cop is enabled
  3. With Ruby 3.0, run rubocop --show-cops Lint/LastKeywordArgument
  4. Verify that cop is disabled

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Peter Leitzen

Merge request reports