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

Improve performance by having boxhead as a data frame. #1896

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented Sep 30, 2024

Summary

It proved more challenging than I thought initially, because of the way R nests objects in data frame vs how tibble does it.

I added a workaround to unnest such objects to improve performance

Related GitHub Issues and PRs

See how the boxhead initalialization takes a full 2 seconds

image

This PR:
image

I have added a few (simpler) tests to make sure this doesn't break (or is easier to debug)

The only remaining thing would be to find a way to avoid resolving columns as this alone takes up a full second of run time. I will think of this.

Checklist

Sorry, something went wrong.

@olivroy
Copy link
Collaborator Author

olivroy commented Sep 30, 2024

The snapshot failure was due to the new katex package I didn't yet have installed. To prevent this issue from happening in the future, I am stripping the katex version from the snapshot. I verified manually that only the katex version changed in the snapshot! The rest is the same

Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM!

@rich-iannone rich-iannone merged commit 10711c5 into rstudio:master Oct 1, 2024
12 checks passed
@olivroy olivroy deleted the boxh branch October 1, 2024 15:01
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.

None yet

2 participants