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

ProjectBranchLines +/-Subject
mediawiki/extensions/MobileFrontendwmf/1.33.0-wmf.16+5 -5mobile.init/editor: Don't create loading overlay if we're not going to use it
mediawiki/extensions/MobileFrontendmaster+5 -5mobile.init/editor: Don't create loading overlay if we're not going to use it
mediawiki/extensions/MobileFrontendmaster+33 -5VE: Load HTML in parallel with modules
mediawiki/extensions/MobileFrontendwmf/1.33.0-wmf.16+33 -5VE: Load HTML in parallel with modules
mediawiki/skins/MinervaNeuemaster+18 -1VE: Load HTML in parallel with modules
mediawiki/extensions/MobileFrontendmaster+16 -27VE: Load all VE modules at the same time
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 added a subscriber: gerritbot.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 added a subscriber: matmarex.

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 added a subscriber: Stashbot.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 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:

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 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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