Page MenuHomePhabricator

Remove top/bottom queue distinction
Closed, ResolvedPublic

Description

Following T107399, the top queue is now asynchronous.

In fast environments this means the page is "loaded" before the startup module arrives and and top queue starts. As such the top and bottom queues may actually execute round about the same time.

There are two aspects here to consider:

Load priority. The separation means some scripts start loading early, and other scripts load late. In between is essentially content resources (e.g. images). With limited bandwidth, regardless of SPDY/HTTP2, there could be value in loading scripts in two stages instead of one so that higher priority scripts (define?) may arrive earlier and don't block images bandwidth-wise. It being asynchronous means it's no longer blocking the parsing and rendering of html, but that doesn't mean we magically doubled the bandwidth available to users.

Execution priority. This is tightly coupled with the previous point. The top queue being asynchronous means first paint can occur before script execution of any kind. However that isn't a requirement. In slow and uncached environments the top and bottom scripts will likely load and execute around the same time. However in fast or cached environments (e.g. repeated visits) the top and bottom scripts do execute at notably different times.

There is also a caveat in that when we previously tried to move the "bottom" queue to start asynchronously at the top of the body, various scripts broke due to the assumption that some scripts would run after the main content is parsed (e.g. dom-ready-ish to have certain elements available). We'll need to identify those and ensure they are bound to $() or DOMContentLoaded accordingly. This will also cause impact on user scripts and gadgets that make bad assumptions about execution time. Though in my experience gadgets already use dom-ready handlers correctly already (especially due to how before 2010 (pre-ResourceLoader) scripts were all at the top. This agrees with the mw-js-deprecate metric that addOnloadHook is by far the most popular method. I suspect the majority of issues with missing dom-ready handlers will come from Wikimedia-maintained extensions and MediaWiki core.

Related Objects

Event Timeline

Krinkle created this task.Aug 21 2015, 3:30 PM
Krinkle updated the task description. (Show Details)
Krinkle raised the priority of this task from to Needs Triage.
Krinkle added subscribers: Krinkle, ori, Catrope.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 21 2015, 3:30 PM
Krinkle triaged this task as Normal priority.Sep 4 2015, 2:54 AM
Krinkle set Security to None.
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.
Krinkle added a subscriber: Peter.Oct 5 2015, 10:37 PM

Aside from the top/bottom distinction for javascript, there is a similar distinction for stylesheets.

However, that distinction was only recently introduced (T97420). Let's see about undoing that first.

@Peter and @ori both found that this extra stylesheet request is hurting render and load time performance. b7c0e537ebb721 attempted to revert this, though that was reverted for unrelated reasons at the time.

ori assigned this task to Krinkle.Oct 20 2015, 6:13 PM
saper added a subscriber: saper.Oct 21 2015, 8:34 AM

Improvement in the "time to first painted" in T107399 is impressive. But do we have a way to measure how many of those "quickly painted" pages actually result in the "flash of unstyled content"?

Also, how does T107399 influence slower network connections? For example, when I was using Tor to access WMF wikis (which is pretty fast those days!) I was getting T115692 every time.

Improvement in the "time to first painted" in T107399 is impressive. But do we have a way to measure how many of those "quickly painted" pages actually result in the "flash of unstyled content"?

To a first approximation, I'm confident no pages have a FOUC. T107399 doesn't affect that. The login and preference pages are isolated edge cases due to how they are uniquely constructed.

There are a few FOUCs mobile due to late loading of styles. However those are caused by late-loading of styles. They are not caused by any asynchronicity in the loading of scripts. This was a regression from T97420. This task (T109837) will resolve that by consolidating the queues back at the top of the page.

Merging the top and bottom script queues (both of which may be async already) will put more load on the top queue. While this is perfectly fine and will naturally scale (thanks to automatic batching and distribution). There is one problem: Cache hits.

The load.php requests created by mw.loader are naturally asynchronous. The startup module itself uses <script async>. However when the startup module is cached, it may execute synchronously during the critical path. On top of that, it will execute localStorage cache hits for required modules synchronously as well.

To avoid a performance regression there I'm gonna block this on T130855: Make mw.loader.store asynchronous.

Krinkle raised the priority of this task from Normal to High.Apr 11 2016, 6:32 PM
saper added a comment.Apr 12 2016, 9:31 PM

What would be the best strategy to pre-select active tab based on the # on the preferences screen without causing the page to reflash (HTML, CSS and then pre-selecting code run in async)?

Gilles added a subscriber: Gilles.Jun 8 2016, 1:13 PM
Krinkle renamed this task from Consider removing top/bottom queue distinction to Remove top/bottom queue distinction.Aug 4 2016, 7:31 PM

Removed T130855 as blocker. After thinking about this some more with @ori, I realised the store is not a dependency. It's fine to fetch items from the store one by one (it's just a plain in-memory object anyway; we only fetch from localStorage once for the whole blob).

Rather, the dependency is that execution of code fetched from mw.loader.store needs to be async (one per item instead of all at once). This is done in mw.loader.work.

T142129: Source code eval should be async in mw.loader.work

He7d3r added a subscriber: He7d3r.Oct 3 2016, 2:44 AM

Change 320315 had a related patch set uploaded (by Krinkle):
resourceloader: Remove top/bottom queue distinction

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

Change 320315 merged by jenkins-bot:
resourceloader: Remove top/bottom queue distinction

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

Krinkle closed this task as Resolved.Nov 17 2016, 9:33 PM