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

Potential forced style layout #348

Open
bakura10 opened this issue Apr 25, 2023 · 5 comments
Open

Potential forced style layout #348

bakura10 opened this issue Apr 25, 2023 · 5 comments

Comments

@bakura10
Copy link

bakura10 commented Apr 25, 2023

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:

rect = el.getBoundingClientRect();

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:

rect = el.getBoundingClientRect();

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):

realImg.src = src;

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.

@bluesmoon
Copy link
Member

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.

@bakura10
Copy link
Author

Thanks for the answer. I will do some more debugging then, because the more <picture> tag I add on the page, the more forced reflows attributed to the getBoundingClientRect appears in Chrome inspector tools. May be a side effect of another code. I will let you know if I find anything :)

@bluesmoon
Copy link
Member

The getBoundingClientRect probably is causing a reflow, but it isn't being invalidated later. Are the picture elements visible when this happens?

@bakura10
Copy link
Author

@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 tag.

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.

@bakura10
Copy link
Author

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:

image

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants