1787072 - Element.getBoundingClientRect performance on CloudFlare mirage script is horrible on pages with lots of images, unlike Chrome.

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Thomas Wisniewski [:twisniewski]]

This bug is a spin-off of the webcompat.com issue: https://github.com/webcompat/web-bugs/issues/108157

Based on my diagnosis there, it seems that Chrome may have added performance optimizations to getBoundingClientRect which make it perform quickly with CloudFlare's Mirage script on image-heavy sites like https://gamepress.gg/feheroes/weapon-skills

I see no evidence of Mirage doing something different in Chrome, so I worry that this significant performance difference may be affecting other sites using Mirage. (It causes a slow script warning on my 2019 Intel macbook in responsive design mode; I can only imagine how long some phones would take to finish the task on Fenix).

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Do you have a profile we could look at? How do I reproduce the issue?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Oh, I think I managed to see it (there's another jank source from an ad script...)

Comment 3

[Thomas Wisniewski [:twisniewski]]

I can make another profile if it will still help, but my profile's time almost all went down a JS stack trace leading to their isWithinViewport function, with all that time basically spent in Element.getBoundingClientRect, and Reflow. I caught it by opening the page in responsive design mode on my macbook, waiting a moment to confirm that the tab was "frozen", starting profiling, then stopping it a few seconds later before the slow script prompt appeared.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1787072 - Avoid useless reframes setting the src attribute of a broken image that already has an image frame. r=#style,#layout-reviewers

Comment 5

[Emilio Cobos Álvarez (:emilio)]

This is not about optimizing getBoundingClientRect or what not. The page is doing img.src = ...; img.getBoundingClientRect();, and we're invalidating too much after the src setter which cause getBoundingClientRect() to reflow.

There are two issues here. The first one is that that invalidation is useless in this case. That's fixed by comment 4. The other is that setting src should trigger an async image load (bug 1076583). That's a bit more tricky / annoying to fix, I suspect.

Comment 6

[Thomas Wisniewski [:twisniewski]]

For the record I also see getBoundingClientRect performance possibly affecting another webcompat bug today, based on the performance profile at https://github.com/webcompat/web-bugs/issues/107388. I'm not sure if they're the same issue however, based on that profile spending the time in Styles/Style computation rather than in Reflow.

Comment 7

[Thomas Wisniewski [:twisniewski]]

Just for the record, I tried a local build with and without this patch, and I couldn't reproduce the slow script warning with the patch applied (testing in responsive design mode). So this should definitely help on the original issue, at least.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f26214c64a4
Avoid useless reframes setting the src attribute of a broken image that already has an image frame. r=dholbert

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3f26214c64a4