-
Notifications
You must be signed in to change notification settings - Fork 293
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
Potential forced style layout #348
Comments
Line 861 doesn't update the DOM. It operates on an in-memory image object that isn't attached to the DOM. It's only used for srcsets. |
Thanks for the answer. I will do some more debugging then, because the more |
The getBoundingClientRect probably is causing a reflow, but it isn't being invalidated later. Are the picture elements visible when this happens? |
@bluesmoon , yes, all the picture element are inside a slideshow. For animation purpose, each slide is using visibility: hidden (so probably considered as "visible" as the space is allocated). Each slide contains a Chrome inspector reports me one reflow per slide. So with six slides, I get 6 reflows reported on the getBoundingClientRect in Boomerang, which quickly adds up, performance wise. I will do further debugging on my end. |
For information if this may helped for debugging @bluesmoon , here is the store where I am facing this issue: https://prestige-theme-allure.myshopify.com/ You can see in the inspector tool a succession of recalculation and forced layout in quick succession in Boomerang code: However, I found some code that may be not optimal in Shopify side as well, but I am having a hard time trying to find the root cause of this here. |
Hi :),
We are creating themes on Shopify platform, which uses internally Boomerang for its performance measuring.
While looking for performance bottlenecks, I found Chrome reporting several "forced recalculation" and "forced layout", that were all attributed to Boomerang and specifically this part:
boomerang/plugins/restiming.js
Line 833 in e7f702e
I am not super familiar with Boomerang code base and how it is supposed to work, but reading the code seems to indicate that, inside a for loop, you are both asking the browser dimensions:
boomerang/plugins/restiming.js
Line 833 in e7f702e
And then invalidate by writing the DOM here (we are using the
<picture>
tag extensively, and it seems that because of this this condition is always true):boomerang/plugins/restiming.js
Line 861 in e7f702e
This would therefore cause as many invalidations as you have items in the for loop. In our case, Boomerang seems to cause a bit of overhead due to this.
The read should probably be batched, and then all the images should be updated after all the reading has happened, to avoid those forced reflows.
But, as said, I am not familiar enough with this codebase, there might be reasons this is done this way.
The text was updated successfully, but these errors were encountered: