1522288 - Should we start image load using microtask and not Gecko's current stable state (== end-of-task)?

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

Description

[Markus Stange [:mstange]]

There's an interesting bit in the profile from bug 1522282: http://bit.ly/2W8Djfr

In this profile, the user is looking at an empty screen for over a second while the network is being completely idle. Then, around timestamp 4.2 seconds, a whole bunch of image loads are kicked off. If you look at the stacks on the content process main thread around that time, you can see that the image loads are kicked off from the following stack:

mozilla::CycleCollectedJSContext::ProcessStableStateQueue
mozilla::dom::ImageLoadTask::Run
mozilla::dom::HTMLImageElement::LoadSelectedImage
nsImageLoadingContent::LoadImage
nsContentUtils::LoadImage
imgLoader::LoadImage

In other words, the image load is delayed until the content process main thread gets a chance to run an ImageLoadTask.
Usually, the main thread is really busy running other stuff. In my opinion we should fire off the image loads much earlier, as soon as we know that the image is needed.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

That is stable state, not a real task. stable states run in Gecko at the end of current task.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Are we somehow queuing ImageLoadTask too late.?

Comment 3

[Markus Stange [:mstange]]

Ah, I see. That makes it less bad. Still, the current task is a really long task in this case, and starting the loads even in the middle of the task might be preferable.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

IIRC, HTML specification defines when img loading is supposed to start, but then, no browser quite follows the spec regarding stable state handling.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

https://github.com/whatwg/html/issues/4188 has some background

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

https://www.w3.org/Bugs/Public/show_bug.cgi?id=24724#c21 is also related

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

https://html.spec.whatwg.org/multipage/images.html#reacting-to-dom-mutations:update-the-image-data
->
https://html.spec.whatwg.org/multipage/images.html#when-to-obtain-images
->
https://html.spec.whatwg.org/multipage/images.html#update-the-image-data

Comment 8

[Boris Zbarsky [:bzbarsky]]

Related is bug 1318316.

That said:

1. I think we should start image loads from microtasks, yes. Especially since that's what Chrome does.

2. We may need to switch other things (e.g. the HTMLImageElement.decode method) from stable state (in the current Gecko definition) to microtasks at the same time.

3. This may not help bug 1522282, if there's JS running through the whole task, because there won't be a microtask checkpoint either, right?

Comment 9

[Bas Schouten (:bas.schouten)]

Based on profiles, it looks like Chrome fires off the Image Loads directly from a JS function that gets run off an rAF callback. The network activity shows the image requests being initiated while the JS is running.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Does Chrome fire off an image load for "something" if you do something like this:

img.src = "something";
var start = new Date();
while (new Date() - start < 5000);
img.src = "something-else";

?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: img_load_start.html

I see only one load start, both in Chrome and Nightly - as expected.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #8)>

3. This may not help bug 1522282, if there's JS running through the whole
   task, because there won't be a microtask checkpoint either, right?

Image loading seem to start because of some raf callbacks, so reflow and painting end up being in the same task too. Microtask checkpoint is at the end of each raf callback.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

Just a quick wip.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Base: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ef3412e88b856e7ff693017b3081580fe1c1746
patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=52065c24d1780c3e7696dcfdd37621a29ecb7832

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Hard to see any meaningful changes in raptor6.

Comment 16

[Sean Feng [:sefeng]]

Attachment: Bug 1522288 - Start using microtask for img loading and img decode r=smaug

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc7c8fe8588f
Start using microtask for img loading and img decode r=smaug

Comment 18

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/dc7c8fe8588f