Rubocop: Performance: avoid avoidable `File.directory?`

Created on 24 May 2020  路  3Comments  路  Source: rubocop-hq/rubocop

We spend 3% of the time calling File.directory? when we know it's a file and not a directory...

$ stackprof tmp/stackprof-cpu-*.dump --method 'RuboCop::ConfigStore#for'
RuboCop::ConfigStore#for (/Users/mal/rubocop/lib/rubocop/config_store.rb:32)
  samples:   860 self (2.7%)  /    865 total (2.7%)
  callers:
     765  (   88.4%)  RuboCop::Runner#inspect_file
      52  (    6.0%)  RuboCop::ResultCache#file_checksum
      20  (    2.3%)  RuboCop::Runner#file_offense_cache
      13  (    1.5%)  RuboCop::Runner#get_processed_source
      10  (    1.2%)  RuboCop::ResultCache.cache_root
       5  (    0.6%)  RuboCop::ResultCache.allow_symlinks_in_cache_location?
  callees (5 total):
       5  (  100.0%)  RuboCop::ConfigLoader.configuration_file_for
  code:
                                  |    32  |     def for(file_or_dir)
    1    (0.0%) /     1   (0.0%)  |    33  |       return @options_config if @options_config
                                  |    34  | 
  859    (2.7%) /   859   (2.7%)  |    35  |       dir = if File.directory?(file_or_dir)
                                  |    36  |               file_or_dir
                                  |    37  |             else
                                  |    38  |               File.dirname(file_or_dir)
                                  |    39  |             end
    5    (0.0%)                   |    40  |       @path_cache[dir] ||= ConfigLoader.configuration_file_for(dir)
                                  |    41  |       path = @path_cache[dir]

Most helpful comment

@marcandre Done :)

All 3 comments

@tejasbubane Oh, I'm sure. That's why I wrote "avoid avoidable calls" (not all calls :-) ). But when we call it from the top 4 callers, we know they are file paths.

One possibility would be to create a base method for_dir, and have the current for and a new method for_file call for_dir. Then the top 4 callers above (at least) can call for_file. The two cases you found could call for_dir directly too (and Dir.pwd could even be the default value of the argument). Would you like to have a go at it?

@marcandre Done :)

Was this page helpful?
0 / 5 - 0 ratings