From 5a9d2231c80d244c019f238587d0b5ae4f3ae7a4 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 28 Aug 2018 04:26:28 +0100 Subject: [PATCH] resourceloader: Don't defer discovery of critical request via rIC() == Overview This reverses a small part of the big dec800968eb commit. That commit eliminated the 2nd of 3 JS requests required on a page. The first JS request is now modules=startup with the registry and mw.loader. The main JS request for modules (now 2nd) is triggered from RLQ, which fires when startup.js calls 'startUp()'. == Regression In commit dec800968eb, I thought I was being smart by letting the browser "take a breath" at the end of startup.js before calling 'startUp()'. I now believe that this artifical is a mistake. I thought it was a good idea because, before dec800968eb startup.js made an http request (for jquery). During that network request the main thread was free to do rendering. Then, when jquery came back, we called startUp() to trigger our main request for modules (3rd, now 2nd). While this request no longer exists, the idea was to keep that thread yield at the end to startup.js. I now think it was a mistake because requestIdleCallback is not a "small" thread yield. Rather, I now find in production that in the majority of cases (all mobile, and desktop first views) the rIC is not called until *after* the page load event fires. This means that during the entire page rendering time, the network and javascript thread were sitting idle. Prioritising rendering and making the page load complete 0.5s faster is great. But, underutilising the network and delaying time to JS interation by 1-2 seconds, is bad. == Cost The startUp() function is quite light. All it does is a bunch of pure JavaScript processing (no DOM), and then creates 1 script tag On desktop (MacBook) with 6x CPU slowdown and mobile emulation (enwiki/Obama) it clocks startUp() at 35ms. On a real mobile device (Nexus 5, Android 4.4, the oldest currently supported Android version/device I could find on BrowserStack) it clocks startUp() on enwiki/Obama at 44ms. This seems small enough to make it worth doing right away, without artificial delay, so that it can kick off the network request in the background while we're rendering - which is exactly how things were before commmit dec800968eb was merged. This means the 'load' event will once again include the main JS request. After dec800968eb, this request unintentionally started after the 'load' event. I'm hoping that with the eliminated request for jquery+mediawiki.base, we'll still keep some of the winnings we had from a much earlier 'load' event. Bug: T192623 Change-Id: I4942bfd236c72b0cf4c23b0e2a0d5e5e069c0de0 --- resources/src/startup/startup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/src/startup/startup.js b/resources/src/startup/startup.js index 7e7fd6a4a6..ee72166072 100644 --- a/resources/src/startup/startup.js +++ b/resources/src/startup/startup.js @@ -145,5 +145,5 @@ window.isCompatible = function ( str ) { // This embeds mediawiki.js, which defines 'mw' and 'mw.loader'. $CODE.defineLoader(); - mw.requestIdleCallback( startUp ); + startUp(); }() ); -- 2.20.1