Page MenuHomePhabricator

Consider disabling data URI embedding in CSSMin for ResourceLoader
Closed, DeclinedPublic

Description

Even on cached page views and repeat views, rendering takes a fair amount of time. @Peter suspects the parsing of data URIs to be a possible bottleneck. Especially on mobile.

This is non-trivial to undo since we rely on these to some extend to avoid flashes of unstyled content (e.g. hover states, dialogs etc.). Normally, when browsers load stylesheets, referenced images are not downloaded until after the selector applies (at which point it's kind of too late). We may be able to work around this by using preloading or prefetching, however.

Let's start by investigating a static copy of our page with and without data URI embedding enabled in a stylesheet and run it through WebPageTest and others to compare rendering.

See also:

"Extreme Web Performance for Mobile Devices", Maximiliano Firtman, at Velocity 2015 Amsterdam
http://www.slideshare.net/firt/extreme-web-performance-for-mobile-devices-54478615#slide=154

Details

Related Gerrit Patches:

Event Timeline

Krinkle created this task.Nov 12 2015, 10:24 PM
Krinkle raised the priority of this task from to Needs Triage.
Krinkle updated the task description. (Show Details)
Krinkle added subscribers: Krinkle, Peter, ori, Catrope.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptNov 12 2015, 10:24 PM

Change 230286 had a related patch set uploaded (by Krinkle):
Do not embed images in file modules as data URIs

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

Krinkle renamed this task from CSSMin: Consider removing data URI embedding in stylesheets to Consider disabling data URI embedding in CSSMin for ResourceLoader.Nov 12 2015, 10:40 PM
Krinkle set Security to None.
Krinkle removed a project: Patch-For-Review.
ori added a comment.Nov 16 2015, 7:55 PM

@Krinkle / @Peter to discuss next steps.

ori triaged this task as Medium priority.Nov 16 2015, 7:56 PM
ori moved this task from Inbox to Backlog: Small & Maintenance on the Performance-Team board.
ori added a comment.Nov 30 2015, 10:37 AM

Raising priority. At the very least, there should be no data URIs in above-the-fold CSS. @Krinkle, could you take this on?

ori raised the priority of this task from Medium to High.Nov 30 2015, 10:38 AM
Krinkle claimed this task.Dec 3 2015, 6:26 PM
Krinkle moved this task from Backlog to Accepted Enhancement on the MediaWiki-ResourceLoader board.

Change 257887 had a related patch set uploaded (by Krinkle):
[WIP] resourceloader: Disable CSSMin data URI embedding

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

Change 257888 had a related patch set uploaded (by Krinkle):
[WIP] resourceloader: Disable CSSMin data URI embedding

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

Change 257888 abandoned by Krinkle:
[WIP] resourceloader: Disable CSSMin data URI embedding

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

Change 257887 abandoned by Krinkle:
[WIP] resourceloader: Disable CSSMin data URI embedding

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

Krinkle added a comment.EditedDec 9 2015, 6:43 PM

@Peter and I did some research today with https://gerrit.wikimedia.org/r/257887 applied to test.wikipedia.org and comparing the before and after in WebPageTest from various browsers and devices.

We previously felt like the browser (especially on mobile devices) were spending a lot of time parsing, processing, and other synchronously allocating image objects for images found in data URIs in the stylesheet.

However after removing all data URIs and replacing them with regular urls that point to the same resource, we found no notable speed gain. Not on desktop/Chrome and not on a real Android device.

A few observations:

  • Bandwidth gained. The main CSS request went from 12K to 8K (presumably due to omission of embedded data URIs). Naturally some of these images were now requested separately (amount to about 1.5K). That's still a 2,5K win, presumably due to no longer loading images that aren't used and/or from the lack of base64 inflation.
  • loadEnd regressed slightly. There is a longer tail of requests in the waterfall. Despite HTTP/2, the tail is longer because these requests start much later. This is an inevitable problem because browsers (by design) only request images referenced by CSS for style rules that apply to elements on the page, which is only known after said elements are parsed.
  • Content images earlier. Images used in the content (e.g. the first main image of the article) are requested earlier and rendered earlier. Previously it requested the stylesheet first, but now in most cases it requests the images first. They download in parallel as before. But the first non-blank render used to be 52% (article without logo and no image), but is now 73% (image included in first render).

Other WebPageTest runs:

1https://phabricator.wikimedia.org/T118514
2
3Tests without any change (Before)
4
5Chrome
6http://www.webpagetest.org/result/151209_DR_REX/
7 http://www.webpagetest.org/video/compare.php?tests=151209_DR_REX-r:1-c:0 (median first view)
8
9iPhone
10http://www.webpagetest.org/result/151209_YH_RFJ/
11
12Moto G
13http://www.webpagetest.org/result/151209_AY_RJS/
14 http://www.webpagetest.org/result/151209_AY_RJS/3/details/ (fastest first view)
15 http://www.webpagetest.org/video/compare.php?tests=151209_AY_RJS-r:3-c:0
16
17After removing data URIs
18
19Chrome
20http://www.webpagetest.org/result/151209_6R_SPT/
21 http://www.webpagetest.org/video/compare.php?tests=151209_6R_SPT-r:3-c:0 (median first view)
22 Mostly the same
23
24iPhone
25http://www.webpagetest.org/result/151209_C6_SPZ/
26
27Moto G
28http://www.webpagetest.org/result/151209_EC_SK7/
29 http://www.webpagetest.org/result/151209_EC_SK7/4/details/ (fastest first view)
30 http://www.webpagetest.org/video/compare.php?tests=151209_EC_SK7-r:4-c:0
31
32
33Conclusions:
34Saves bandwidth (12K > 8KB for main css request). Even with the extra image requests included (~0.5K), because many weren't used.
35Longer tail of image requests. Requests start relatively late (CSS applies, indirection). Final paint regresses.
36On mobile [confirmation needed] content images are requested and rendered earlier (inside the first paint even, previously first paint was without the infobox image). Saves a reflow as well.
37At the cost of rendering interface icons later.
38Maybe keep @embed for the handful of icons that are on every page view. And consider converting to something like @push (T???).
39
40
41Bugs:
42 * Mobile doesn't reserve image box width/height in infobox, causing reflow. Fine on desktop.

Desktop:

http://www.webpagetest.org/video/compare.php?tests=151209_DR_REX-r:1-c:0 (before)


http://www.webpagetest.org/video/compare.php?tests=151209_6R_SPT-r:3-c:0 (after)


  • Before: Text and icons (e.g. lock badge) are in first render. Image and rest in final render.
  • After: Text in first render. Icons, image and rest in final render.

Both cases, first render is at the same point, so we didn't get text earlier. Regression.

Mobile:

http://www.webpagetest.org/video/compare.php?tests=151209_AY_RJS-r:3-c:0 (before)



http://www.webpagetest.org/video/compare.php?tests=151209_EC_SK7-r:4-c:0 (after)

  • Before: Text and icons (e.g. hamburger menu) in first render. Infobox image in final render.
  • After: Text and infobox image in first render. Icons in final render.

Having the image in the first render is nice, but in "after" first render was actually 0.3 seconds later. (from 1.9s to 2.2s). So it actually delayed rendering of text (and extension of the blank canvas) by 300ms. But at the gain of having the image in the first render. But with the side-effect of icons popping in much later (potentially distracting).

Either way, the final render ("settled") was also later still. So a loss in almost all ways, except for the small bandwidth gain in the CSS payload.

Action items:

  • Removing data URIs globally doesn't seem to have measurable positive impact. We should not consider embedding to be an anti-pattern.
  • Saving bandwidth for unused resources is still worth pursuing. We should audit individual uses of @embed and consider removing them from places not commonly used above the fold on first render.
  • Removing from from highly-visible areas (e.g. above the fold icons on every page) would cause a regression both in perceived performance and actual NavTiming metrics. Instead, we should keep @embed in those cases for the time being and consider using HTTP/2 push for those in the future (T120984).

Change 230286 abandoned by Krinkle:
Do not embed images in file modules as data URIs

Reason:
Moving conversation back to T118514 on Phabricator.

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

Krinkle closed this task as Declined.Dec 17 2015, 12:11 AM

Closing per above conclusion. Opened T121730 for the audit.

Volker_E added a comment.EditedJan 4 2016, 1:49 AM

@Krinkle I'm wondering if your test cases for WebPageTest were first load results? When encoding in base64, the issue stated in this heatedly discussed article from mid 2013:

Regardless of whether the data URI content exists in a cached CSS or HTML file, the browser must decode the image each time a page renders: the decoding cost is paid repeatedly every time a page is viewed.

First off, it's not clear how appropriate the data from the article is nowadays, mid 2013 is a quite a while back. Also, for SVG icons we don't use base64. Only URL encoding is used due to performance reasons. SVG rendering (URL decoding?) speed might have increased significantly in recent mobile browsers.