Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize visiting code #979

Merged
merged 3 commits into from
Aug 21, 2022
Merged

Optimize visiting code #979

merged 3 commits into from
Aug 21, 2022

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Aug 21, 2022

For #558 and #759.

This is a tad faster and enables cleaner stack traces, in particular recursive calls now follow each other directly, without an interleaving map(). We now can profile, flatten recursive calls, and get a clean picture where time is actually spent.

@codecov-commenter
Copy link

Codecov Report

Merging #979 (7031c22) into main (2fa7369) will decrease coverage by 0.02%.
The diff coverage is 90.69%.

@@            Coverage Diff             @@
##             main     #979      +/-   ##
==========================================
- Coverage   90.12%   90.10%   -0.03%     
==========================================
  Files          47       47              
  Lines        2664     2698      +34     
==========================================
+ Hits         2401     2431      +30     
- Misses        263      267       +4     
Impacted Files Coverage Δ
R/initialize.R 96.96% <ø> (ø)
R/relevel.R 47.82% <50.00%> (-0.07%) ⬇️
R/visit.R 94.84% <92.30%> (-5.16%) ⬇️
R/nested-to-tree.R 96.29% <100.00%> (ø)
R/transform-block.R 100.00% <100.00%> (ø)

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 d8a7301 is merged into main:

  •   :rocket:cache_applying: 26.8ms -> 24ms [-11.02%, -9.94%]
  •   :rocket:cache_recording: 1.11s -> 1.09s [-2.33%, -1.26%]
  •   :rocket:without_cache: 2.96s -> 2.88s [-2.85%, -2.09%]

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

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Aug 21, 2022

Nice, speed benchmarks support your claim. I am sold 🥳

@lorenzwalthert lorenzwalthert merged commit 4075099 into main Aug 21, 2022
@lorenzwalthert lorenzwalthert deleted the f-558-speed branch August 21, 2022 07:10
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.

3 participants