search

akamai / boomerang

End user oriented web performance testing and beaconing
http://akamai.github.io/boomerang/
Other
1.86k stars 292 forks source link

Potential forced style layout #348

Open bakura10 opened 1 year ago

bakura10 commented 1 year ago

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: https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L833

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: https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L833

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): https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L861

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

bakura10 commented 1 year ago

@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 commented 1 year ago

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.