Page MenuHomePhabricator

Audit use of @embed and remove where not needed (2019)
Closed, ResolvedPublic

Description

T233675: Audit startup JavaScript payload (Aug 2018—Sept 2019) | T233676: Audit default page modules payload (2019)


Objective

This task proposes to focus on the following theme for a quarter: Reduce use of @embed to only images that generally render above-the-fold.

Expected impact, based on "Size of stylesheets" at https://www.mediawiki.org/wiki/Wikimedia_Performance_Team/Page_load_performance:

  • Improved visual progression for readers (first paint earlier, last paint same or better as today).
Rationale

Smaller stylesheets download quicker. The sooner the styles arrive to completion, the sooner the browser is allowed to start rendering the above-the-fold HTML it already got in its buffer.

In 2010, we decided to embed images because of:

  • how browsers worked at the time and our understanding of how browsers worked,
  • the use of HTTP/1 (relatively high cost for each additional request, limited number of connections that each can only start requesting the next resource after the previous has gotten its complete response),
  • our focus on reducing the total time taken and bandwidth spent for loading a page.

Embedding images allowed the images to act like a super-sprite in terms of compression (saving on bytes transferred) and reduced the number of requests (fewer roundtrips, thus faster page load time). Between then and 2019, some things have changed:

  • we now use HTTP/2, in which parallel requests have fewer disadvantages, less overhead, and there is no longer a concurrency limit after which requests would be stalled. There is still non-zero overhead and saving requests can still benefit. But, its overhead is now smaller, and as such there are now also scenarios in which an extra request would actually save time rather than increase time. It's also important to note that we spent fewer bytes on HTTP in HTTP/2 due to the ability to compress values over multiple requests (HPACK).
  • we still consider bandwidth cost an important factor. But for a given individual and a given quantity of bytes on the network, it is now cheaper than it was ten years ago (citation needed). Hence, while 10 KB may've been a deal-breaker in some trade offs before, a similar trade off might now only be justified for 30 KB. Depending on how many bytes a change saves or adds, and how much it improves other factors (e.g. UX, perceived performance, rendering speed, battery power, etc.), it may now be acceptable to be less optimal on bytes if it means a significant improvement on other factors.
  • several browsers now support HTTP Preload.
  • we now have much better insight into visual rendering in browsers (RUM: Paint Timing. WebPageTest: Visual metrics). And we've decided to make them part of the key performance metrics we monitor.

The way browsers work, and have worked even in 2010, is that a stylesheet blocks the appearance of any content or layout on the page. The flow is as follows:

  1. Browser requests HTML page from server, and
  2. Browser starts to receive HTML from server.
  3. Browser detects sub resources, such as stylesheets, images, and scripts. And requests them. Stylesheets are given a high priority through the HTTP/2 semantics.
  4. Browser continues to receive HTML from the server and parses it, but may not yet render.
  5. Browser starts to receive the stylesheet.
  6. Browser receives the remaining parts of the stylesheet.
  7. Browser may now render the HTML it's parsed so far. (First paint)
  8. Browser continues to receive HTML and will progressively parse/render in chunks until it reached the end of the HTML stream. The same applies to images and scripts.

By embedding icons in stylesheets, no page content, text, or layout may appear until all icons are downloaded. This is in essence a sort of "ultra high priority" hack, analogous to synchronous scripts (and we know how much we don't like synchronous scripts). The use of embedding is therefore imho unacceptable, without exception. No icon can be more important than the layout of the website, and the article text itself.

By letting our icons load separately (in parallel with preload) I hope that the first paint (above-the-fold content and layout) can occur sooner than today.

The paint with the icons would still be at the same time as today, in browsers supporting Preload. In other browsers (shrinking population over time) icons would appear 1 RTT later than today, but still before the load event, given other loadevent-tracked sub resources taking at least 2 RTT as well.

In retrospect, this isn't really anything new. Most any major site today works the same way. GitHub, Google properties, Twitter, Facebook. They load at once with content and layout in their final resting place, but icons pop in later. Unlike layout FOUCs, these seem quite acceptable and not fundamentally different to how other images render in a browser. That is, the website logo or infobox images also appear separate from the text. Same for native applications as well.

Best practices
  • If used in a styles-only module, the entire article will remain blank until the embedded image has been downloaded and processed. No text, no skin layout, nothing will appear. This the the absolute strongest form of blocking and critical path available on the web. As such, it requires equally strong rationale before using :)

    Also, the styles queue varies between page-views which means users may have to download the same embedded icons multiple times. Without embedding, they enjoy the benefits of browser caching.
  • If used in a JS module that also contains styles, the module will not be "ready" until the image is downloaded. This delays execution of other modules that depend on it, as well as any mw.loader promise, until the styles are downloaded, parsed, and added to the page (regardless of whether the CSS rules match).

    This means if a module is lazy-loaded on-demand only where it is definitely needed, and the selector for the background image will definitely match, then using @embed does not waste bandwidth. But, you might still want to avoid it. For example, if a JS module contains a complex dialog framework that is only loaded when you are about to create a dialog, do you want the dialog to be visible sooner without the image first, or block any use of the dialog on the image being downloaded? If the dialog would look broken or incomplete without the image, then embedding might make sense. But, otherwise, best to avoid it still and allow progressive rendering.
Plan
  • Remove unneeded use of @embed in MediaWiki core modules and WMF-deployed extensions.
  • Determine whether we can improve the internals of @embed for the remaining use cases.
    • Cost of parsing data URIs: Analyse the cost in browsers on various mobile devices / and OSes for downloading and parsing stylesheets with data URIs vs regular urls. Safari and Firefox were once known to parse the image binary data forcefully while processing the stylesheet, instead of skipping the data URI to only parse it when the CSS rule matches (like browsers already do for regular urls).
    • Compressibility of data URIs: Even if data URIs are around the same length as the path URL, the path urls should compress much better in practice given most of the paths are similar, with only the base name differing. This should very significantly shrink icon packs.
  • If useful, implement @preload
  • Migrate from @embed to @preload.
  • Coordinate roll out with performance survey (@Gilles), and attempt to A/B test.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 456524 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Remove @embed from wikibase.client.css background-image

https://gerrit.wikimedia.org/r/456524

Krinkle updated the task description. (Show Details)Mar 5 2019, 4:22 PM
Krinkle removed a project: Patch-For-Review.
Krinkle added a subscriber: Gilles.
Krinkle renamed this task from Audit use of @embed and remove where not needed to Audit use of @embed and remove where not needed (2019).Apr 24 2019, 6:52 PM
Krinkle removed Krinkle as the assignee of this task.Jun 24 2019, 11:09 PM

(Pushed to next quarter)

Krinkle added a comment.EditedJul 22 2019, 8:10 PM
Distribution of effective connection speed

From the netinfo API (spec). For two dates in June and July 2019 (data only available for Android devices and Chrome desktop).

Without the unknowns:

# a Monday in June 2019
hive (event)> SELECT event.netinfoEffectiveConnectionType, COUNT(*) FROM navigationtiming WHERE year=2019 AND month=6 AND day=24 AND event.isOversample=0 GROUP BY event.netinfoEffectiveConnectionType;
netinfoeffectiveconnectiontype  _c1
slow-2g 687
2g      657
3g      13916
4g      162837
NULL    128833


# a Saturday in July 2019
hive (event)> SELECT event.netinfoEffectiveConnectionType, COUNT(*) FROM navigationtiming WHERE year=2019 AND month=7 AND day=20 AND event.isOversample=0 GROUP BY event.netinfoEffectiveConnectionType;
slow-2g 715
2g      733
3g      13804
4g      146505
NULL    127017
Krinkle raised the priority of this task from Low to High.EditedJul 25 2019, 6:54 PM

(Raise priority to reflect status change from future goal to current quarterly goal.)

Change 525840 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] [WIP] resources: Remove @embed where it may do more harm than good

https://gerrit.wikimedia.org/r/525840

Krinkle updated the task description. (Show Details)Aug 5 2019, 3:28 PM
Krinkle updated the task description. (Show Details)Aug 6 2019, 1:37 PM

Change 528480 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/skins/Vector@master] [WIP] Disable embedding of tab background images

https://gerrit.wikimedia.org/r/528480

Krinkle added a comment.EditedAug 6 2019, 3:21 PM
MobileFrontend / Minerva

Currently loads ~ 2KB as blocking/embedded images. 6 icons (2KB) logged-out, and 8 icons (2.6KB) when logged-in. Most of them seem to be for the navigation menu, which is actually hidden by default. Probably not worth embedding / delaying the rendering of articles for.

The only icon not for the menu is the chevron arrow, which is used in the last-modified footer at the end of the page (far below the fold), also doesn't seem beneficial to embed.

Change 528499 had a related patch set uploaded (by Krinkle; owner: VolkerE):
[mediawiki/skins/Vector@master] [WIP] Replace raster image gradients with CSS

https://gerrit.wikimedia.org/r/528499

Volker_E updated the task description. (Show Details)Aug 6 2019, 4:12 PM
Volker_E added a subscriber: Volker_E.
Jdlrobson claimed this task.Aug 6 2019, 7:11 PM
Jdlrobson removed Jdlrobson as the assignee of this task.
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Jdlrobson.
Krinkle updated the task description. (Show Details)Aug 6 2019, 7:12 PM
Krinkle claimed this task.Aug 24 2019, 5:21 PM

Change 342661 restored by Krinkle:
Remove @embed for image not used during page load

https://gerrit.wikimedia.org/r/342661

Change 532098 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/MultimediaViewer@master] [WIP] Remove @embed where it needlessly delays loading of the UI

https://gerrit.wikimedia.org/r/532098

Change 532269 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/WikiLove@master] Remove PNG fallback for JS-only interface

https://gerrit.wikimedia.org/r/532269

Krinkle updated the task description. (Show Details)Sep 2 2019, 5:37 PM

Change 539422 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.mixins: Rename .background-image-svg to .background-image-svg-embed

https://gerrit.wikimedia.org/r/539422

Change 539422 abandoned by Krinkle:
mediawiki.mixins: Rename .background-image-svg to .background-image-svg-embed

Reason:
Nevermind. It's easy to revert if the result is negative. Might as well switch all at once centrally through the mixin.

https://gerrit.wikimedia.org/r/539422

Change 342661 merged by Krinkle:
[mediawiki/extensions/UniversalLanguageSelector@master] Remove @embed where it needlesslay delays the page load

https://gerrit.wikimedia.org/r/342661

Change 532098 merged by Krinkle:
[mediawiki/extensions/MultimediaViewer@master] Remove @embed where it needlessly delays loading of the UI

https://gerrit.wikimedia.org/r/532098

Change 532269 merged by Krinkle:
[mediawiki/extensions/WikiLove@master] Remove PNG fallback for JS-only interface

https://gerrit.wikimedia.org/r/532269

Change 539582 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/UniversalLanguageSelector@master] ext.uls: remove redundant -webkit gradient/svg reference

https://gerrit.wikimedia.org/r/539582

Change 528480 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Remove @embed page load delays (and misc optimisations)

https://gerrit.wikimedia.org/r/528480

Change 540005 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/Vector@master] Replace portal break image by SVG gradient

https://gerrit.wikimedia.org/r/540005

Change 540005 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Replace portal break image by SVG gradient

https://gerrit.wikimedia.org/r/540005

Change 540205 had a related patch set uploaded (by Krinkle; owner: VolkerE):
[mediawiki/skins/Vector@wmf/1.34.0-wmf.25] Replace portal break image by SVG gradient

https://gerrit.wikimedia.org/r/540205

Change 540205 merged by jenkins-bot:
[mediawiki/skins/Vector@wmf/1.34.0-wmf.25] Replace portal break image by SVG gradient

https://gerrit.wikimedia.org/r/540205

Change 540219 had a related patch set uploaded (by Krinkle; owner: VolkerE):
[mediawiki/skins/Vector@wmf/1.34.0-wmf.25] Replace raster image gradients with CSS where easily applicable

https://gerrit.wikimedia.org/r/540219

Change 528499 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Replace raster image gradients with CSS where easily applicable

https://gerrit.wikimedia.org/r/528499

Change 525840 merged by jenkins-bot:
[mediawiki/core@master] resources: Remove @embed where it may do more harm than good

https://gerrit.wikimedia.org/r/525840

Change 540248 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@wmf/1.34.0-wmf.25] resources: Remove @embed where it may do more harm than good

https://gerrit.wikimedia.org/r/540248

Change 540219 merged by jenkins-bot:
[mediawiki/skins/Vector@wmf/1.34.0-wmf.25] Replace raster image gradients with CSS where easily applicable

https://gerrit.wikimedia.org/r/540219

Change 540248 merged by jenkins-bot:
[mediawiki/core@wmf/1.34.0-wmf.25] resources: Remove @embed where it may do more harm than good

https://gerrit.wikimedia.org/r/540248

Change 540746 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove data URI fallback for IE6 and IE7

https://gerrit.wikimedia.org/r/540746

Change 540746 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove data URI fallback for IE6 and IE7

https://gerrit.wikimedia.org/r/540746

Krinkle closed this task as Resolved.Sat, Oct 5, 12:38 AM

As of about 1.5 day ago, this is deployed on all Wikipedias (Thu Oct 3, 18:30 UTC).

Summary of changes:

  1. Data URI embedding of icons was removed in favour of http url as background image. This should improve page load times on desktop, because the Vector skin used this technique in its layout for several icons and images (on all pages).
  2. Some of the images in Vector (desktop skin) were removed entirely in favour of CSS gradients.
  3. URLs for auto-generated icon variants now have long-cache headers as well (T233343). Regular urls to static files have had long-cache headers for years, but for auto-generated images from load.php this was previously missing. This should improve page load times on mobile, where the Minerva skin uses this technique (Minerva disabled icon embedding several months ago already, so these urls weren't just IE fallbacks but used by modern browsers as well).

Dashboard links for past week to compare:

  • MediaWiki Static router – This is the router that proxies the static SVG and PNG files. Most of these were previously embedded with the fallback URL targeting IE6-7 only.
    • Difference: No notable difference in backend traffic. I imagine there'd be a notable increase in frontend traffic for "verified" urls (from mobile), but we don't have stats for that unfortunately.
  • Navigation Timing by platform (mobile)
    • Difference: On the first day after the deployment, all percentiles have reached a new record in fast page loads on mobile. Given this new lowest-point-of-day was not reached on the day itself but a day after, suggests it might due to Change #3 (icon long-caching). Before this change, the unversioned icon urls were cached for 24 hours only, whereas now they are considered immutable (allowing 30+ days of unconditional cache reuse). As such, I would indeed expect this impact (if significant) be visible for repeat views after the first day.
    • The loadEvent.mobile.p95 metric is about 0.3s lower than usual, and briefly dipped below 4s which hadn't been seen before (looked back until 2017).
    • The loadEvent.mobile.p50 metric about 50ms lower than usual (could be noise), but did dip below 800ms, which was last seen in January.
  • Navigation Timing by platform (desktop)
    • Difference: No notable difference found.
      • It's unfortunate that we didn't see much impact from using native gradients vs data URIs. In isolation we found small improvements but I guess in practice it's negligible with all the complexity of the main article content and JS payload being much more significant. The Vector layout and JS was pretty minimal already.
      • It is good to see that there was no negative impact from making the Vector and ULS icons loaded by URL instead of embedded. It would've been nice to see an improvement, but I was more worried that it would cause a regression due to request overhead and slightly later discovery, but looks like our optimisations balanced it out for as far as it needed balancing out.
  • Synthetic testing (WebPageTest, WebPageReplay):
    • Difference: Could not find any notable differences. However, I would like to see the synthetic data for repeat views to see if we can confirm the above impact we saw on mobile. However, I'm unable to find those graphs right now (ref. T234138).
(Unannotated version: F30549868)

Change 539582 merged by jenkins-bot:
[mediawiki/extensions/UniversalLanguageSelector@master] ext.uls: remove redundant -webkit gradient/svg reference

https://gerrit.wikimedia.org/r/539582