improve speed of RuboCop::PathUtil#smart_path #5120

walf443 · 2017-11-24T15:34:13Z

Benchmark

Warming up --------------------------------------
original relative_path
                        11.762k i/100ms
improve without range object
                        12.092k i/100ms
improve without range object with caching Dir.pwd
                       174.293k i/100ms
Calculating -------------------------------------
original relative_path
                        129.932k (± 3.5%) i/s -    658.672k in   5.075475s
improve without range object
                        135.333k (± 3.8%) i/s -    677.152k in   5.011012s
improve without range object with caching Dir.pwd
                          3.533M (± 7.7%) i/s -     17.604M in   5.026444s

Comparison:
improve without range object with caching Dir.pwd:  3532795.2 i/s
improve without range object:   135332.8 i/s - 26.10x  slower
original relative_path:   129932.1 i/s - 27.19x  slower

benchmark script is followings:

require 'benchmark/ips'

Benchmark.ips do |x|
  path = File.join(Dir.pwd, "hoge.rb")
  x.report ("original relative_path") do
    base_dir = Dir.pwd
    path[(base_dir.length + 1)..-1] if path.start_with?(base_dir)
  end

  x.report ("improve without range object") do
    base_dir = Dir.pwd
    len = base_dir.length
    path[len + 1, path.length - len - 1] if path.start_with?(base_dir)
  end

  pwd = Dir.pwd
  x.report ("improve without range object with caching Dir.pwd") do
    base_dir = pwd
    len = base_dir.length
    path[len + 1, path.length - len - 1] if path.start_with?(base_dir)
  end
  x.compare!
end

Why RuboCop::PathUtil#smart_path should improve performance?

This is a patch for profiling rubocop with stackprof

diff --git bin/rubocop bin/rubocop
index abf88369e..0f2406641 100755
--- bin/rubocop
+++ bin/rubocop
@@ -5,12 +5,15 @@ $LOAD_PATH.unshift("#{__dir__}/../lib")

 require 'rubocop'
 require 'benchmark'
+require 'stackprof'

 cli = RuboCop::CLI.new
 result = 0

-time = Benchmark.realtime do
-  result = cli.run
+StackProf.run(mode: :cpu, out: 'tmp/stackprof-cpu-rubocop.dump') do
+  time = Benchmark.realtime do
+    result = cli.run
+  end
 end

 puts "Finished in #{time} seconds" if cli.options[:debug]
diff --git rubocop.gemspec rubocop.gemspec
index 4e0278ceb..db5d539da 100644
--- rubocop.gemspec
+++ rubocop.gemspec
@@ -44,5 +44,6 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency('unicode-display_width', '~> 1.0', '>= 1.0.1')

   s.add_development_dependency('bundler', '~> 1.3')
+  s.add_development_dependency('stackprof')
 end
 # rubocop:enable Metrics/BlockLength

This is output of stackprof result of running ./bin/rubocop on master

$ bundle exec stackprof tmp/stackprof-cpu-rubocop.dump
==================================
  Mode: cpu(1000)
  Samples: 22980 (0.07% miss rate)
  GC: 1505 (6.55%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      2543  (11.1%)        2543  (11.1%)     RuboCop::PathUtil#relative_path
      1505   (6.5%)        1505   (6.5%)     (garbage collection)
      2510  (10.9%)        1454   (6.3%)     Parser::Lexer#advance
      2680  (11.7%)        1434   (6.2%)     RuboCop::AST::Node#each_child_node
       979   (4.3%)         979   (4.3%)     block (2 levels) in <class:Node>
       914   (4.0%)         804   (3.5%)     RuboCop::ConfigStore#for
       782   (3.4%)         782   (3.4%)     Parser::Source::Buffer#slice
       731   (3.2%)         731   (3.2%)     #<Module:0x00007fb78aa44900>.fu_mkdir
       687   (3.0%)         687   (3.0%)     AST::Node#to_a
       541   (2.4%)         379   (1.6%)     Parser::Source::Buffer#line_for_position
       611   (2.7%)         324   (1.4%)     RuboCop::Cop::Cop#cop_config
       306   (1.3%)         287   (1.2%)     RuboCop::Cop::RSpec::Cop#rspec_pattern
       437   (1.9%)         204   (0.9%)     AST::Node#initialize
       202   (0.9%)         202   (0.9%)     Parser::Source::Range#initialize
       177   (0.8%)         177   (0.8%)     #<Module:0x00007fb78d138548>.of
       233   (1.0%)         174   (0.8%)     Parser::AST::Node#assign_properties
       307   (1.3%)         166   (0.7%)     Parser::Source::Buffer#line_for
       149   (0.6%)         149   (0.6%)     RuboCop::ResultCache#valid?
       228   (1.0%)         145   (0.6%)     RuboCop::Cop::Cop#cop_name
       152   (0.7%)         144   (0.6%)     RuboCop::Cop::VariableForce#scanned_node?
       144   (0.6%)         144   (0.6%)     RuboCop::Cop::Cop#initialize
       143   (0.6%)         143   (0.6%)     Parser::Source::Buffer#line_begins
       124   (0.5%)         124   (0.5%)     Parser::Lexer::Literal#coerce_encoding
       120   (0.5%)         120   (0.5%)     RuboCop::AST::Node#parent
       117   (0.5%)         117   (0.5%)     RuboCop::Cop::Cop.badge
      1058   (4.6%)         113   (0.5%)     RuboCop::Cop::Commissioner#remove_irrelevant_cops
       258   (1.1%)         113   (0.5%)     Parser::Source::Buffer#column_for_position
       488   (2.1%)         110   (0.5%)     RuboCop::Cop::VariableForce#dispatch_node
      1299   (5.7%)         107   (0.5%)     RuboCop::AST::Node#visit_descendants
       353   (1.5%)         104   (0.5%)     RuboCop::Config#for_cop
-----------------

RuboCop::PathUtil#relative_path is hotspot.

bundle exec stackprof tmp/stackprof-cpu-rubocop.dump --method 'RuboCop::PathUtil#relative_path'
RuboCop::PathUtil#relative_path (/Users/yoshimin/src/github.com/bbatsov/rubocop/lib/rubocop/path_util.rb:8)
  samples:  2543 self (11.1%)  /   2543 total (11.1%)
  callers:
    2543  (  100.0%)  RuboCop::PathUtil#smart_path
  code:
                                  |     8  |     def relative_path(path, base_dir = Dir.pwd)
                                  |     9  |       # Optimization for the common case where path begins with the base
                                  |    10  |       # dir. Just cut off the first part.
 2543   (11.1%) /  2543  (11.1%)  |    11  |       return path[(base_dir.length + 1)..-1] if path.start_with?(base_dir)
                                  |    12  |
RuboCop::PathUtil#relative_path (/Users/yoshimin/src/github.com/bbatsov/rubocop/lib/rubocop/path_util.rb:8)
  samples:     2 self (0.0%)  /      2 total (0.0%)
  callers:
       2  (  100.0%)  RuboCop::Config#path_relative_to_config
  code:
                                  |     8  |     def relative_path(path, base_dir = Dir.pwd)
                                  |     9  |       # Optimization for the common case where path begins with the base
                                  |    10  |       # dir. Just cut off the first part.
    2    (0.0%) /     2   (0.0%)  |    11  |       return path[(base_dir.length + 1)..-1] if path.start_with?(base_dir)
                                  |    12  |

100% called by RuboCop::PathUtil#smart_path.

After this patch, stackprof result changed followings:

==================================
  Mode: cpu(1000)
  Samples: 28366 (0.05% miss rate)
  GC: 2001 (7.05%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      4000  (14.1%)        2101   (7.4%)     RuboCop::AST::Node#each_child_node
      2001   (7.1%)        2001   (7.1%)     (garbage collection)
      3324  (11.7%)        1808   (6.4%)     Parser::Lexer#advance
      1533   (5.4%)        1533   (5.4%)     block (2 levels) in <class:Node>
      1164   (4.1%)        1164   (4.1%)     Parser::Source::Buffer#slice
      1030   (3.6%)        1030   (3.6%)     AST::Node#to_a                                                                                                                                                                                          859   (3.0%)         756   (2.7%)     RuboCop::ConfigStore#for                                                                                                                                                                                718   (2.5%)         718   (2.5%)     #<Module:0x00007f9c7d80c368>.fu_mkdir                                                                                                                                                                   777   (2.7%)         553   (1.9%)     Parser::Source::Buffer#line_for_position                                                                                                                                                                776   (2.7%)         454   (1.6%)     RuboCop::Cop::Cop#cop_config
       365   (1.3%)         342   (1.2%)     RuboCop::Cop::RSpec::Cop#rspec_pattern
       286   (1.0%)         286   (1.0%)     Parser::Source::Range#initialize
       614   (2.2%)         283   (1.0%)     AST::Node#initialize
       259   (0.9%)         259   (0.9%)     #<Module:0x00007f9c7ddb82d0>.of
       238   (0.8%)         238   (0.8%)     Parser::Lexer::Literal#coerce_encoding
       331   (1.2%)         235   (0.8%)     Parser::AST::Node#assign_properties
       419   (1.5%)         223   (0.8%)     Parser::Source::Buffer#line_for
       236   (0.8%)         219   (0.8%)     RuboCop::Cop::VariableForce#scanned_node?
       199   (0.7%)         199   (0.7%)     RuboCop::Cop::Cop#initialize
       198   (0.7%)         198   (0.7%)     Parser::Source::Buffer#line_begins
       277   (1.0%)         184   (0.6%)     RuboCop::Cop::Cop#cop_name
       166   (0.6%)         166   (0.6%)     RuboCop::Cop::IgnoredNode#ignored_nodes
       359   (1.3%)         164   (0.6%)     Parser::Source::Buffer#column_for_position
       156   (0.5%)         156   (0.5%)     AST::Node#eql?
       150   (0.5%)         150   (0.5%)     RuboCop::Cop::Cop.badge
      1246   (4.4%)         137   (0.5%)     RuboCop::Cop::Commissioner#remove_irrelevant_cops
      8710  (30.7%)         136   (0.5%)     RuboCop::Cop::Commissioner#on_send
       686   (2.4%)         133   (0.5%)     RuboCop::Cop::VariableForce#dispatch_node
      1946   (6.9%)         133   (0.5%)     RuboCop::AST::Node#visit_descendants
       129   (0.5%)         129   (0.5%)     RuboCop::AST::MethodDispatchNode#setter_method?

Before submitting the PR make sure the following are checked:

Drenmi · 2017-11-24T17:53:22Z

Nice! 👍

I think rebasing on latest master should fix the CI failure.

pocke · 2017-11-25T10:31:17Z

Awesome improvement!

Fix a rubocop's offense to fix the build failure, and resolve the conflict.
Thank you!

walf443 · 2017-11-25T11:48:03Z

@pocke sorry. fixed it.

travis-ci/pr seems to failed followings:

$ bundle exec rake documentation_syntax_check generate_cops_documentation
Files:         458
Modules:        76 (   12 undocumented)
Classes:       430 (    2 undocumented)
Constants:     666 (  661 undocumented)
Attributes:     23 (    0 undocumented)
Methods:      1019 (  850 undocumented)
 31.12% documented
* generated /home/travis/build/bbatsov/rubocop/manual/cops_bundler.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_gemspec.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_layout.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_lint.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_metrics.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_naming.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_performance.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_rails.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_security.md
* generated /home/travis/build/bbatsov/rubocop/manual/cops_style.md
git diff --quiet manual
git diff manual
diff --git a/manual/cops_rails.md b/manual/cops_rails.md
index 0c7a4af..4f076b6 100644
--- a/manual/cops_rails.md
+++ b/manual/cops_rails.md
@@ -203,11 +203,11 @@ create_table :users do |t|
 end
 
 
-### Important attributes
+### Configurable attributes
 
-Attribute | Value
---- | ---
-Include | db/migrate/\*.rb
+Name | Default value | Configurable values
+--- | --- | ---
+Include | `db/migrate/*.rb` | Array

Is it related this PR?

walf443 · 2017-11-25T11:52:13Z

I send PR to fix this https://github.com/bbatsov/rubocop/pull/5124/files

Drenmi · 2017-11-25T20:03:25Z

@bbatsov: This was accidentally closed by GitHub. 🙂

walf443 force-pushed the improve_path_util_smart_path branch 2 times, most recently from 0fbdc89 to 6ff2866 Compare November 24, 2017 15:55

walf443 force-pushed the improve_path_util_smart_path branch from 6ff2866 to ce3d18e Compare November 25, 2017 02:50

walf443 force-pushed the improve_path_util_smart_path branch 2 times, most recently from 540bdcf to 14cc493 Compare November 25, 2017 11:04

improve speed of RuboCop::PathUtil#smart_path

7ec1ff8

walf443 force-pushed the improve_path_util_smart_path branch from 14cc493 to 7ec1ff8 Compare November 25, 2017 11:27

walf443 mentioned this pull request Nov 25, 2017

update manual/cops_rails by rake generate_cops_documentation #5124

Merged

11 tasks

pocke approved these changes Nov 25, 2017

View reviewed changes

garettarrowood approved these changes Nov 25, 2017

View reviewed changes

bbatsov closed this in #5124 Nov 25, 2017

walf443 mentioned this pull request Nov 26, 2017

improve speed of RuboCop::PathUtil#smart_path #5137

Merged

11 tasks

dependencies bot mentioned this pull request Dec 12, 2017

Update rubocop in / from 0.51.0 to 0.52.0 chauncey-garrett/dotfiles#48

Open

dependencies bot mentioned this pull request Jan 1, 2018

Update rubocop in / from 0.51.0 to 0.52.1 podemos-info/census#119

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve speed of RuboCop::PathUtil#smart_path #5120

improve speed of RuboCop::PathUtil#smart_path #5120

walf443 commented Nov 24, 2017 •

edited

Loading

Drenmi commented Nov 24, 2017

pocke commented Nov 25, 2017

walf443 commented Nov 25, 2017

walf443 commented Nov 25, 2017

Drenmi commented Nov 25, 2017

improve speed of RuboCop::PathUtil#smart_path #5120

improve speed of RuboCop::PathUtil#smart_path #5120

Conversation

walf443 commented Nov 24, 2017 • edited Loading

Benchmark

Why RuboCop::PathUtil#smart_path should improve performance?

Drenmi commented Nov 24, 2017

pocke commented Nov 25, 2017

walf443 commented Nov 25, 2017

walf443 commented Nov 25, 2017

Drenmi commented Nov 25, 2017

walf443 commented Nov 24, 2017 •

edited

Loading