1844432 - On the Speedometer 3 "NewsSite" subtests, HTMLImageElement spends too much time creating URLs

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Markus Stange [:mstange]]

On the Speedometer3 subtest "NewsSite-Next", one of our biggest buckets for improvement is the time under Node.appendChild. This function spends most of its time creating nsIURI objects under HTMLImageElement::LoadSelectedImage.

Eliminating the performance difference in Node.appendChild should improve our score on this subtest by 1.7%.

You can run this test at https://shell--speedometer-preview.netlify.app/?suite=NewsSite-Next&iterationCount=1 .

Profiles:
Firefox: https://share.firefox.dev/3OkdCp4 (634 samples)
Chrome: https://share.firefox.dev/44LYYfM (176 samples, 3.6x faster)

The image URL is placeholder_light.jpg.

I think what's happening is that the actual load gets kicked off from setAttribute("src", "..."). Creating a URL there is fine. But then, when we add the node to the document, we end up in HTMLImageElement::BindToTree, which adds a script runner for HTMLImageElement::MaybeLoadImage (with aAlwaysLoad == false).

This results in a call to HTMLImageElement::LoadSelectedImage, which creates a new URI, notices that it matches the currently loaded source, and bails out.

Can we avoid creating URI objects in this case?

Comment 1

[Markus Stange [:mstange]]

This issue also affects "NewsSite-Nuxt" ("u" instead of "e"), which would be improved by 2.7% if this were fixed.

Comment 2

[Markus Stange [:mstange]]

Boris, is this something you could look into? (I noticed you looked at similar code in bug 1789475)

Comment 3

[Boris Chiou [:boris]]

I'm not sure if it is possible to check if it matches the current source if we don't create the URI. For responsive image, we still have to select the correct image, so it may be pretty hard to skip the selection.

For creating URI object, I'm not sure if there are any other ways to simplify the check the matchness. Emilio, do you have any idea to avoid creating URI objects to improve this?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I mean, in theory we can cache the parsed src attribute right? It probably makes sense.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1844432 - Cleanups to HTMLImageElement. r=smaug,edgar,boris

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1844432 - Cache parsed src attribute URI. r=smaug,edgar,boris

Depends on D184265

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58914a204fb0
Cleanups to HTMLImageElement. r=smaug

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/58914a204fb0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc55ec4ceb86
Cache parsed src attribute URI. r=smaug

Comment 10

[Norisz Fay [:noriszfay]]

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