Increase the priority of images loaded using CSS
Categories
(Core :: Layout, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: smaug, Assigned: heycam)
Details
Attachments
(2 files, 2 obsolete files)
By default images are loaded using low priority.
If I read the code correctly, images bound to <img> may then get higher priority
https://searchfox.org/mozilla-central/rev/2f1020dc4176d38dd5f3d0496f3c46849862ee0b/layout/generic/nsImageFrame.cpp#367
https://searchfox.org/mozilla-central/rev/2f1020dc4176d38dd5f3d0496f3c46849862ee0b/layout/generic/nsImageFrame.cpp#2087
But I don't see similar happening to images loading via CSS.
As an example https://www.huffingtonpost.com/ has the main image loaded as css background.
Filter https://img.huffingtonpost.com/asset/5c85d874240000f404a2c6d1.jpeg?cache=i1o3kdw5eu&ops=2160_720,prog,quality_75
in https://perfht.ml/2EPRwpN
That image is loaded with low priority (10).
Many other images are loaded with a tiny bit higher priority 8, and are loaded before the main image.
(-20 is the highest priority)
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
smaug, the page has changed and doesn't have a big main content CSS background-image any more. Do you have other similar sites you could use to test this patch? Or would you prefer me to just get it reviewed and landed? I spot checked locally on a couple of pages that the priorities get boosted for CSS background-images.
Reporter | ||
Comment 2•5 years ago
|
||
The main image is still
style="background-image: url(https://img.huffingtonpost.com/asset/5c92f6972300007d006c4276.png?cache=hvzjao0a46&ops=960_720,prog,quality_75);" attribute in a <div> element.
One argument for prioritizing <img> over CSS images (particularly background-image/border-image) is that <img> can affect the layout of the page, so loading can cause things to move around. (This is particularly true for prioritization of images that are offscreen.)
Assignee | ||
Comment 4•5 years ago
|
||
The existing prioritization boost of HTML <img> will be greater than the CSS images with this patch, for exactly that reason. (It doesn't use the CATEGORY_SIZE_QUERY boost category for CSS images.)
Assignee | ||
Comment 5•5 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #2)
The main image is still
style="background-image: url(https://img.huffingtonpost.com/asset/5c92f6972300007d006c4276.png?cache=hvzjao0a46&ops=960_720,prog,quality_75);" attribute in a <div> element.
Oh, I didn't realize I was being redirected to the Australian edition of the site, which doesn't have the big banner background image.
Assignee | ||
Comment 6•5 years ago
|
||
(In reply to David Baron :dbaron: 🏴 ⌚UTC-7 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it) from comment #3)
One argument for prioritizing <img> over CSS images (particularly background-image/border-image) is that <img> can affect the layout of the page, so loading can cause things to move around. (This is particularly true for prioritization of images that are offscreen.)
But that makes me realize that list-style-image images should use that category.
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
Olli I don't know how to evaluate the effectiveness of this patch. What do you suggest?
Reporter | ||
Comment 9•5 years ago
|
||
I would at least profile https://www.huffingtonpost.com/ page load. But perhaps the patch won't be enough to help there, since there are other images which are usually loaded before the main image.
Assignee | ||
Comment 10•5 years ago
|
||
I didn't see any significant change in load ordering on that page.
Profile without patch: https://perfht.ml/2URtBNf
Profile with patch: https://perfht.ml/2CBbcND
I wrote a small test to verify that the priority changing does cause a CSS background-image that's about to be painted to start its load before others that are blocked due to HTTP connection limits: https://mcc.id.au/2019/image-load/
That's on my server using HTTP 1.1. I don't really know how HTTP 2 is meant to work, which is what huffpost.com uses.
Assignee | ||
Comment 11•5 years ago
|
||
When I updated my server to use HTTP 2 all the resources in my test loaded simultaneously. That does look more like pattern when loading huffpost.com, where the initial batch of image loads from img.huffingtonpost.com (28 of them, including this banner image) start loading at the same time.
Assignee | ||
Comment 12•5 years ago
|
||
This is what the first batch of loads from img.huffingtonpost.com looks like. The highlighted one is the banner image.
Assignee | ||
Comment 13•5 years ago
|
||
Of the 28 images that are loading at that point, the nsISupportsPriority priorities of them are:
- 1 with priority 20
- 7 with priority 18
- 8 with priority 8
- 2 with priority -1, one of which is the banner image here.
These get converted into HTTP 2 priority values in the range [1, 40], but it seems like HTTP 2 priorities are intended to be interpreted as proportions, not as groups where more important groups get absolute priority. mt tells me that servers are inconsistent in their implementation of priorities anyway, so there's probably not much we can do here other than select a priority and hope the server does something useful with it.
So with that in mind I'll request review of my patch but don't expect it to make a difference on this site.
Comment hidden (obsolete) |
Assignee | ||
Comment 15•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 16•5 years ago
|
||
Comment 17•5 years ago
|
||
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7f0d03ec917c Boost loading priority of CSS images similarly to how we do for HTML images. r=tnikkel
Comment 18•5 years ago
|
||
bugherder |
Description
•