speed up make_object_linter - ref issue #226

[russHyde]

As described in #226 , the speed of the object_name_... linters has reduced since introduction of the object_name_linter("snake_case") style linters

The function make_object_linter was refactored. Some conditional logic, that checked whether a given object was an operator, a generic or a base-function and bypassed that object if it was, was moved out of an lapply loop into a prefiltering step. This step is vectorised over all object-names in a given file, so speeds up linters that are built using make_object_linter.

No additional unit-tests were added

For analysis of the check.R file referenced in #318 using a) the CRAN version (1.0.2); b) the current master github version; c) the updated version

code:

system.time(lint("helpers/check.R", linters = object_length_linter(20), cache = FALSE))

a) cran:
user system elapsed
40.312 0.156 40.820

b) master (ooft!; my laptop isn't great ..):
user system elapsed
577.812 0.984 586.789

c) update:
user system elapsed
324.437 0.078 326.421
326.063 0.109 327.099

I'll admit the improvement is less than two-fold and comes nowhere near the cran version. I get similar speed--up when comparing lint_package on the lintr source code (the only default_linters that were speeded up were object_length_* and object_name_* and they had equivalent degree of speedup).

Will have a play with other possible changes & try and work out why there's such a big difference between the sys-time speedup (~ 10 fold) and the actual speed up (< 2 fold)

[codecov]

Codecov Report

Merging #356 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #356      +/-   ##
==========================================
+ Coverage    85.7%   85.76%   +0.06%     
==========================================
  Files          41       41              
  Lines        2154     2164      +10     
==========================================
+ Hits         1846     1856      +10     
  Misses        308      308

Impacted Files	Coverage Δ
R/object_name_linters.R	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c9e8db...9259892. Read the comment docs.

[russHyde]

I've updated my code (but have yet to put it on github) - replacing object_name_linter and object_length_linter with versions that are vectorised over all of the the object-names in a file (the above just performs a vectorised prefiltering step).

This reduces the run times further

• object_length is similar to the bulk of the other linters;
• object_name is ~ 2-3 times slower than most other linters but ~ 5x faster than in the master branch

pipe_continuation_linter remains a compute hog.

Should I add the new code to the current pull-request?

# times of default_linters on `check.R` using my work desktop
                             linter  master  vector
1                 assignment_linter  12.752  12.581
2               closed_curly_linter  13.924  13.814
3                     commas_linter  15.093  15.067
4             commented_code_linter  13.273  13.543
5                  equals_na_linter  14.608  14.759
6  function_left_parentheses_linter  18.200  18.600
7               infix_spaces_linter  15.189  15.237
8                line_length_linter  12.719  12.983
9                     no_tab_linter  14.654  14.917
10             object_length_linter 222.020  13.638
11               object_name_linter 224.244  45.003
12              object_usage_linter  22.166  22.355
13                open_curly_linter  13.776  14.089
14         pipe_continuation_linter 178.237 180.176
15             single_quotes_linter  13.970  14.186
16             spaces_inside_linter  14.345  14.601
17   spaces_left_parentheses_linter  18.413  17.653
18      trailing_blank_lines_linter  12.433  13.206
19       trailing_whitespace_linter  14.229  14.614

[jimhester]

I think as far as the object linter goes we should try to use the approach in #236, IIRC it is pretty close to a drop in replacement, but there are a few corner cases that need to be fixed.

[russHyde]

Cool. Thanks for looking into it; the lintr speed was a bit of a problem for me (mostly in vim/syntastic).

[jimhester]

Thanks @russHyde, I will merge this for now, i the future we can consider moving to #236, but this is a good improvement for now.

Great work, this package is now better thanks to you!

[russHyde]

@jimhester thanks for merging. I will try to have a look at your xpath-based code; I didn't quite follow it the first time round (due to my xpath inexperience). If there's any particular aspect of #236 that you want someone to look at, ping me. Very happy to help on lintr, since one of my packages (and my workflow more generally) depends really heavily on it.

I should state that I didn't end up pushing the whole of my work on this PR (the stuff on object_[name|length]_linter that is mentioned above); will have a look when I'm back in the office.