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

Esanders created this task.Nov 8 2018, 12:59 PM
Restricted Application added a project: VisualEditor. · View Herald TranscriptNov 8 2018, 12:59 PM
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 claimed this task.Nov 8 2018, 3:10 PM
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 added a subscriber: matmarex.

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

Scheduled for SWAT: https://wikitech.wikimedia.org/wiki/Deployments#deploycal-item-20190206T1700

(We wanted to have this live like two weeks ago…)

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:

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 closed this task as Resolved.Mar 8 2019, 12:08 AM
ppelberg added a subscriber: ppelberg.

@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:


More investigation needed...
edit: Filed as T217826

ppelberg reopened this task as Open.Mar 8 2019, 12:08 AM

Yes, this part is resolved.

Ok, cool – thanks, @Esanders

ppelberg closed this task as Resolved.Mar 8 2019, 10:35 PM