Boomerang performance issue

[bakura10]

Hi,

This issue does not directly impact Dawn, but is a platform performance problem.

I always suspected Boomerang, the performance measurement tool that Shopify is using, to be a performance bottleneck, quite ironically. But could not prove it, until today.

While trying to find the performance bottlenecks of our latest version of Prestige, I found a lot of consecutive "Forced layout" / "Forced style recalculation" problems. Forced layouts are one of the biggest cause for initial "sluggy" page and is something I work hard to minimize.

Using Chrome performance tools, I found all those forced reflow to be attributed to Boomerang, and more specifically this line: https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L833

Normally, this should cause only one forced reflow, but I had much more, all attributed to the same line.

I dig into the code and found that, a few lines letter, Boomerang actually write the DOM: https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L861

Consecutive reading/writing are typical cause of thrash layout. The reason is that there is, apparently, a condition here that always enter into the condition if the image is inside a <picture> tag, which is the case for us: https://github.com/akamai/boomerang/blob/e7f702e5362e8a8f56f0cf3040ee24077d13a9a8/plugins/restiming.js#L857

So I did a try, by adding a few more slides to our slideshow (which would add more tag), and boom:

Basically, the usage of causes one reflow/thrash layout for every picture element you have on the page. A single slideshow with 4 or 5 images is not unusual, so you have as many reflows as picture.

I raised an issue on Boomerang repo (akamai/boomerang#348) but it seems the repo has a very low activity, and that Shopify may have forked this for their own internal use.

At the very best, this code should be rewritten to batch the read and the write to avoid one reflow per picture, and maybe should be evaluated to check if this code is needed. It seems to be one of the only place in Boomerang repo that query the DOM, and it seems like a bad idea for something aims to measure the performance to potentially introduce that many reflows.

Thanks!

[siakaramalegos]

I could not repeat the same test to get these results. Going to close for now, but @bakura10 if you can get someone to be able to repeat the results on a different computer we can reopen this.