Page MenuHomePhabricator

Load page content in parallel with VE code on Mobile with ArticleTargetLoader
Closed, ResolvedPublic

Description

As shown in T206228, we are waiting ~1s before we start loading the page data/metadata, while the VE code loads. If we load this is parallel like we do on desktop page load times would be greatly improved.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

We should check with @Neil_P._Quinn_WMF that we have reasonable page load timings for Mobile VE before we deploy any improvements.

Change 472461 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/skins/MinervaNeue@master] VE: Load HTML in parallel with modules

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

Change 472466 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] VE: Load all VE modules at the same time

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

Esanders edited projects, added VisualEditor (Current work); removed VisualEditor.
Esanders moved this task from Incoming to Code review on the VisualEditor (Current work) board.

This should result in a very significant improvement to mobile page load times.

Change 472461 abandoned by Esanders:
VE: Load HTML in parallel with modules

Reason:
Squashed into Ib0ff6636a5d0f2033b8952ba26aecce5698db893

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

Change 472466 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] VE: Load all VE modules at the same time

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

matmarex subscribed.

Should probably compare before/after numbers as part of QA. I don't know where they are logged.

Should probably compare before/after numbers as part of QA. I don't know where they are logged.

We now have mobile timing values in the EditAttemptStep data stream—I did some basic checks in T205166#4886384 so it looks like the data is there.

I think this is actually not complete. We forgot half of the patch. https://gerrit.wikimedia.org/r/472461 was abandoned and supposed to be squashed into the other patch https://gerrit.wikimedia.org/r/472466, but AFAICS it was not, and I don't see that code on master.

Change 486193 had a related patch set uploaded (by Bartosz Dziewoński; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] VE: Load HTML in parallel with modules

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

Change 486193 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] VE: Load HTML in parallel with modules

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

Change 488494 had a related patch set uploaded (by Bartosz Dziewoński; owner: Esanders):
[mediawiki/extensions/MobileFrontend@wmf/1.33.0-wmf.16] VE: Load HTML in parallel with modules

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

Change 488494 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.33.0-wmf.16] VE: Load HTML in parallel with modules

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

Mentioned in SAL (#wikimedia-operations) [2019-02-06T17:49:47Z] <thcipriani@deploy1001> Synchronized php-1.33.0-wmf.16/extensions/MobileFrontend: SWAT: [[gerrit:488494|VE: Load HTML in parallel with modules]] T209052 (duration: 00m 57s)

Change 488523 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] mobile.init/editor: Don't create loading overlay if we're not going to use it

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

Change 488523 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] mobile.init/editor: Don't create loading overlay if we're not going to use it

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

Change 488591 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@wmf/1.33.0-wmf.16] mobile.init/editor: Don't create loading overlay if we're not going to use it

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

Change 488591 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.33.0-wmf.16] mobile.init/editor: Don't create loading overlay if we're not going to use it

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

Tagging @matmarex or @DLynch to do the Engineering QA.

Here is a screenshot of a simlulated-slow-3g network waterfall, showing the metadata request (api.php), RESTBase request (api/rest_v1/...) and ResourceLoader request (load.php) all starting in parallel:

image.png (405×1 px, 103 KB)

I wanted to compare the performance using the production data as part of QA:

Should probably compare before/after numbers as part of QA. I don't know where they are logged.

…but actually, we should probably do that in T214450: Analyze impact of parallel loading of page content and mobile visual editor code.

ppelberg subscribed.

@Esanders + @matmarex, is it ok to mark this as "Resolved" considering the unresolved portion of this ticket seems to now be represented in T217826, as mentioned:

Just did a test on live and something weird is happening:

image.png (173×1 px, 41 KB)

More investigation needed...

edit: Filed as T217826