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 DOM operations #2843

Merged
merged 9 commits into from
May 5, 2021
Merged

Conversation

hermansje
Copy link
Contributor

I was testing some settings and edge cases of the PDF printing support using a ~50 slide presentation.
In the Chrome devtools I noticed that the flame chart of a performance recording showed a lot of forced layout spans.
I looked into the areas where this happened and applied the advice to improve time spent on layout:

  • limiting DOM access
  • batching reads at the start of a frame
  • batch writes after reads

When building this library with these changes, and testing again with the presentation I was working with, performance improved and there were less forced layout spans in the performance flame graph.

The code should still do the same functionally.
Promises and requestAnimationFrame were already used elsewhere and async/await gets compiled to promises, so browser compatibility should also remain the same.

Thank you for all the work on this library!

@hermansje
Copy link
Contributor Author

@hakimel after f576b98 there was a merge conflict with this branch. I resolved it, and by doing that I saw Reveal.layoutSlideContents and Reveal.slideContent.layout next to each other. One handles stretch layout, the other handles text fit. Could they be combined in Reveal.slideContent.layout or would that not work in some cases?
Does the current code look good to you? (It might be easiest to review commit by commit.)

@hakimel hakimel merged commit 892c752 into hakimel:dev May 5, 2021
@hakimel
Copy link
Owner

hakimel commented May 5, 2021

This looks great. Thanks a lot @hermansje!

Regarding the layout methods, if I remember correctly the code in Reveal.slideContent.layout only needs to be called once when the slide loads, whereas Reveal.layoutSlideContents needs to be invoked whenever the browser viewport is resized. The naming is confusing—it might be better to rename the prior method to something else.

R0bes pushed a commit to R0bes/Terraform-Presentation that referenced this pull request Jun 7, 2021
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