Skip to content

Multi-line to carry number of lines instead of boolean #782

Merged
lorenzwalthert merged 8 commits intor-lib:masterfrom
lorenzwalthert:issue-776
Apr 30, 2021
Merged

Multi-line to carry number of lines instead of boolean #782
lorenzwalthert merged 8 commits intor-lib:masterfrom
lorenzwalthert:issue-776

Conversation

@lorenzwalthert
Copy link
Collaborator

@lorenzwalthert lorenzwalthert commented Apr 23, 2021

Closes #773.

Here is how the current PR would change benchmark results when merged into master:

  • cache_applying: 0.03 -> 0.03 (12.6%)
  • cache_recording: 1.08 -> 1.09 (1%)
  • without_cache: 3.27 -> 3.23 (-1.4%)

With many int replacements:

  • cache_applying: 0.03 -> 0.03 (0%)
  • cache_recording: 1.19 -> 1.13 (-4.9%)
  • without_cache: 3.57 -> 3.51 (-1.7%)

With them reverted:

  • cache_applying: 0.03 -> 0.03 (-0.6%)
  • cache_recording: 1.12 -> 1.12 (0%)
  • without_cache: 3.44 -> 3.41 (-0.8%)

Ok, I think 1% speed gain is not worth changing every subsetting from x[1] to x[1L]. Readability suffers too much.

@codecov-commenter
Copy link

Codecov Report

Merging #782 (464e57c) into master (073a3ec) will increase coverage by 0.07%.
The diff coverage is 90.11%.

❗ Current head 464e57c differs from pull request most recent head e69e812. Consider uploading reports for the commit e69e812 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #782      +/-   ##
==========================================
+ Coverage   90.55%   90.62%   +0.07%     
==========================================
  Files          47       47              
  Lines        2393     2411      +18     
==========================================
+ Hits         2167     2185      +18     
  Misses        226      226              
Impacted Files Coverage Δ
R/addins.R 15.29% <0.00%> (ø)
R/token-define.R 66.66% <0.00%> (ø)
R/relevel.R 46.15% <41.17%> (ø)
R/io.R 87.75% <50.00%> (ø)
R/utils-navigate-nest.R 80.00% <63.63%> (ø)
R/unindent.R 95.65% <87.50%> (ø)
R/detect-alignment-utils.R 90.47% <89.47%> (ø)
R/communicate.R 92.30% <100.00%> (ø)
R/compat-dplyr.R 85.71% <100.00%> (ø)
R/detect-alignment.R 98.36% <100.00%> (+0.63%) ⬆️
... and 24 more

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 9f9bdff...e69e812. Read the comment docs.

@lorenzwalthert lorenzwalthert merged commit 51bb82f into r-lib:master Apr 30, 2021
@lorenzwalthert lorenzwalthert deleted the issue-776 branch April 30, 2021 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multi_line attribute could carry amount of line breaks instead of just boolean

2 participants