Skip to content

Conversation

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Apr 17, 2023

Avoiding repeated bind_rows() in transformer.

@codecov-commenter
Copy link

Codecov Report

Merging #1112 (1ec1fa8) into main (940010d) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 1ec1fa8 differs from pull request most recent head 6111fef. Consider uploading reports for the commit 6111fef to get more accurate results

@@            Coverage Diff             @@
##             main    #1112      +/-   ##
==========================================
+ Coverage   91.07%   91.09%   +0.01%     
==========================================
  Files          46       46              
  Lines        2712     2717       +5     
==========================================
+ Hits         2470     2475       +5     
  Misses        242      242              
Impacted Files Coverage Δ
R/transform-block.R 100.00% <100.00%> (ø)
R/visit.R 94.94% <100.00%> (+0.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 940010d is merged into main:

  •   :ballot_box_with_check:cache_applying: 203ms -> 203ms [-1%, +1.41%]
  •   :rocket:cache_recording: 942ms -> 898ms [-5.01%, -4.35%]
  •   :rocket:without_cache: 1.95s -> 1.83s [-6.39%, -5.93%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Copy link
Collaborator

@lorenzwalthert lorenzwalthert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solid speed improvement. Can't say no to that :D

@github-actions
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 940010d is merged into main:

  • ❗🐌cache_applying: 311ms -> 1.63s [+422.36%, +426%]
  • ❗🐌cache_recording: 1.48s -> 1.62s [+8.59%, +10.29%]
  •   :rocket:without_cache: 3.1s -> 1.84s [-41.31%, -40.02%]

Further explanation regarding interpretation and methodology can be found in the documentation.

@krlmlr krlmlr force-pushed the f-faster-roundtrip-1 branch from 01845a2 to 1481507 Compare April 17, 2023 14:37
@krlmlr krlmlr requested a review from lorenzwalthert April 17, 2023 14:38
@krlmlr
Copy link
Member Author

krlmlr commented Apr 17, 2023

I have no idea what's wrong with the macOS tests now.

@krlmlr
Copy link
Member Author

krlmlr commented Apr 17, 2023

https://github.com/r-lib/styler/actions/runs/4722467440/jobs/8377340497?pr=1112#step:2:6693 :

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 0c4187c is merged into main:

  •   :ballot_box_with_check:cache_applying: 204ms -> 205ms [-0.52%, +1.76%]
  •   :rocket:cache_recording: 941ms -> 859ms [-9.08%, -8.24%]
  •   :rocket:without_cache: 1.95s -> 1.74s [-11.01%, -10.55%]

@krlmlr
Copy link
Member Author

krlmlr commented Apr 17, 2023

In #1114 now.

@krlmlr krlmlr closed this Apr 17, 2023
@lorenzwalthert lorenzwalthert deleted the f-faster-roundtrip-1 branch April 17, 2023 20:08
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.

4 participants