Page MenuHomePhabricator
Create Task
Maniphest T209052

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

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.

Details

SubjectRepoBranchLines +/-
mobile.init/editor: Don't create loading overlay if we're not going to use itmediawiki/extensions/MobileFrontendwmf/1.33.0-wmf.16+5 -5
mobile.init/editor: Don't create loading overlay if we're not going to use itmediawiki/extensions/MobileFrontendmaster+5 -5
VE: Load HTML in parallel with modulesmediawiki/extensions/MobileFrontendmaster+33 -5
VE: Load HTML in parallel with modulesmediawiki/extensions/MobileFrontendwmf/1.33.0-wmf.16+33 -5
VE: Load HTML in parallel with modulesmediawiki/skins/MinervaNeuemaster+18 -1
VE: Load all VE modules at the same timemediawiki/extensions/MobileFrontendmaster+16 -27
Customize query in gerrit

Related Objects
Search...

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
Esanders added a subscriber: nshahquinn-wmf.Nov 8 2018, 1:03 PM

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

Esanders mentioned this in T206228: Analyse load time performance of FA-length pages on mobile.Nov 8 2018, 2:45 PM
gerritbot subscribed.Nov 8 2018, 2:46 PM

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

gerritbot added a project: Patch-For-Review.Nov 8 2018, 2:46 PM
gerritbot added a comment.Nov 8 2018, 3:03 PM

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.

• marcella awarded a token.Nov 13 2018, 7:45 PM
gerritbot added a comment.Nov 26 2018, 7:33 PM

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

Reason:
Squashed into Ib0ff6636a5d0f2033b8952ba26aecce5698db893

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

gerritbot added a comment.Jan 15 2019, 7:58 PM

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.14; 2019-01-22).Jan 15 2019, 8:00 PM
matmarex moved this task from Code review to Engineering QA on the VisualEditor (Current work) board.Jan 22 2019, 1:51 AM
matmarex subscribed.

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

matmarex added a comment.Jan 23 2019, 5:29 PM

(Neil filed T214450: Analyze impact of parallel loading of page content and mobile visual editor code to investigate)

nshahquinn-wmf added a comment.Jan 23 2019, 9:27 PM
In T209052#4897647, @matmarex wrote:

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.

matmarex moved this task from Engineering QA to In progress on the VisualEditor (Current work) board.Jan 24 2019, 12:13 AM

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.

gerritbot added a comment.Jan 24 2019, 12:21 AM

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

matmarex moved this task from In progress to Code review on the VisualEditor (Current work) board.Jan 24 2019, 12:33 AM
gerritbot added a comment.Feb 6 2019, 2:14 AM

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

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

matmarex moved this task from Code review to Engineering QA on the VisualEditor (Current work) board.Feb 6 2019, 3:31 AM
gerritbot added a comment.Feb 6 2019, 4:33 PM

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

matmarex added a comment.Feb 6 2019, 4:47 PM

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

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

gerritbot added a comment.Feb 6 2019, 5:34 PM

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

Stashbot subscribed.Feb 6 2019, 5:49 PM

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)

gerritbot added a comment.Feb 6 2019, 6:52 PM

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

gerritbot added a comment.Feb 6 2019, 9:30 PM

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

Jdlrobson mentioned this in T215469: [Bug] Closing editor overlay shows loading spinner indefinitely.Feb 6 2019, 9:49 PM
gerritbot added a comment.Feb 6 2019, 10:11 PM

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

gerritbot added a comment.Feb 7 2019, 12:22 AM

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

Stashbot added a comment.Feb 7 2019, 12:47 AM

Mentioned in SAL (#wikimedia-operations) [2019-02-07T00:47:14Z] <twentyafterfour> syncing commit rEMFRdd8654ac9b3f for Bug: T209052

Esanders added a subscriber: DLynch.Feb 13 2019, 6:01 PM

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)

matmarex moved this task from Engineering QA to Product owner review on the VisualEditor (Current work) board.Feb 18 2019, 5:52 PM

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

In T209052#4897647, @matmarex wrote:

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.

matmarex removed a subtask: T214450: Analyze impact of parallel loading of page content and mobile visual editor code.Feb 18 2019, 5:53 PM
matmarex added a parent task: 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 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:

In T214450#5006713, @Esanders wrote:

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

ppelberg reopened this task as Open.Mar 8 2019, 12:08 AM
Esanders added a comment.Mar 8 2019, 6:39 PM

Yes, this part is resolved.

ppelberg added a comment.Mar 8 2019, 10:34 PM

Ok, cool – thanks, @Esanders

ppelberg closed this task as Resolved.Mar 8 2019, 10:35 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits