1874765 - "Notify images" shows up as .3% of SP3

Status: NEW

(Core :: Graphics: ImageLib, task)

Description

[Jeff Muizelaar [:jrmuizel]]

I suspect the majority of these notifications are from the data urls in https://github.com/WebKit/Speedometer/blob/main/resources/todomvc/todomvc-css/src/css/todo-item.css

https://share.firefox.dev/3vxOcNP

It seems like we could avoid doing these notifications because presumably we're just going to paint them synchronously anyways right?

Comment 1

[Timothy Nikkel (:tnikkel)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)

It seems like we could avoid doing these notifications because presumably we're just going to paint them synchronously anyways right?

Images being potentially shared by more than one user complicates things. If we can know that there is just the one consumer of the notifications then we could probably simplify.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

(In reply to Timothy Nikkel (:tnikkel) from comment #1)

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)

It seems like we could avoid doing these notifications because presumably we're just going to paint them synchronously anyways right?

Images being potentially shared by more than one user complicates things. If we can know that there is just the one consumer of the notifications then we could probably simplify.

We know that it's not shared if it's a data url right?

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

A few points:

• I could be wrong, but I believe that data URI images are only sync metadata decoded, not sync decoded, achieved through [1] and [2].
• SchedulePaint is a significant part of currently required work done through these notifications. I wonder if we could schedule async resource updates for images in general, like we do for animated images, and remove these actions entirely.
• If we could reduce the dependencies these events for display, then perhaps it would be work exploring adding desired event masks for the imgRequestProxy listener, so that we can avoid bubbling up at all. Or even removing the listeners entirely.
• Even if we were sync decoding, I'm not convinced that would reduce the burden since the signalling itself doesn't appear to have too much overhead. We would need to reduce what we actually do on these events.

[1] https://searchfox.org/mozilla-central/rev/1aa61dcd48e128a8cbfbe59b7ba43d31bd3c248a/image/ImageFactory.cpp#75
[2] https://searchfox.org/mozilla-central/rev/1aa61dcd48e128a8cbfbe59b7ba43d31bd3c248a/image/imgRequest.cpp#705

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Also, perhaps to add to the confusion, we will sometimes sync decode data URI images through the common use of FLAG_SYNC_DECODE_IF_FAST:
https://searchfox.org/mozilla-central/rev/1aa61dcd48e128a8cbfbe59b7ba43d31bd3c248a/image/DecodedSurfaceProvider.cpp#206

If the encoded image data is > 16384 bytes, it will async decode. So small data URIs will actually sync decode.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

It's worth noting that these are SVG images.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #5)

It's worth noting that these are SVG images.

Once all of the data comes in, those are painted into a surface on the main thread. There is image.svg.blob-image that moves this work to the WebRender blob image path, but there are some edge case bugs that prevented us from shipping. That said, the profile doesn't show us spending much time in SVGDrawingCallback.

Comment 7

[Timothy Nikkel (:tnikkel)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)

(In reply to Timothy Nikkel (:tnikkel) from comment #1)

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)

It seems like we could avoid doing these notifications because presumably we're just going to paint them synchronously anyways right?

Images being potentially shared by more than one user complicates things. If we can know that there is just the one consumer of the notifications then we could probably simplify.

We know that it's not shared if it's a data url right?

data url's are also shared AFAICT, I just double checked, two pages referencing the same data url in the same process only create one RasterImage, it could also be two references to the data url in the same doc.

Comment 8

[Timothy Nikkel (:tnikkel)]

(In reply to Andrew Osmond [:aosmond] (he/him) from comment #6)

(In reply to Jeff Muizelaar [:jrmuizel] from comment #5)

It's worth noting that these are SVG images.

Once all of the data comes in, those are painted into a surface on the main thread. There is image.svg.blob-image that moves this work to the WebRender blob image path, but there are some edge case bugs that prevented us from shipping. That said, the profile doesn't show us spending much time in SVGDrawingCallback.

All issues that I could find via bugzilla with image.svg.blob-image have either been fixed, or I have a patch for that just needs a test to land.