Page MenuHomePhabricator

When section editing, do not build whole CE tree
Closed, ResolvedPublic

Description

Client-side section editing (T76541) works by loading the whole document data and building the whole document model (DM) tree, but only building and attaching a DOM tree for a single branch of that tree (the branch at the "attachedRoot").

This prevents the editing view from being slowed and cluttered by the presence of extraneous content (outside the attachedRoot). Building, attaching and scrolling DOM nodes is expensive, so the computational speedup is substantial.

Conceptually between the logical DM tree and the rendered DOM tree lies the CE tree. As of T76541 we build the whole CE tree, which in principle is wasteful. This task is to instead build only the CE branch at the attachedRoot.

Details

Related Gerrit Patches:
mediawiki/extensions/VisualEditor : masterUpdate VE core submodule to master (213b79783)
VisualEditor/VisualEditor : masterReplace unrendered parts of the view tree with a stub node

Event Timeline

dchan created this task.Feb 10 2019, 9:18 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 10 2019, 9:18 AM
JTannerWMF moved this task from To Triage to Up next on the VisualEditor board.Feb 12 2019, 2:46 PM

Some performance numbers for context: T206228#4731711

This task represents half of "build CE view". So where we estimated a drop from 1000ms to 100ms, our current implementation has only done half of that, so down to ~550ms. This task would get use the rest of the way to 100ms.

Change 522594 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Replace unrendered parts of the view tree with a stub node

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

Esanders added a comment.EditedJul 13 2019, 12:35 PM

Monkey patched on live and edited a section of a long article (mobile site on desktop) (https://en.m.wikipedia.org/wiki/Megan_Rapinoe)

Testing second load times (i.e with the code and page data already fetched and cached), time to loaded was ~2300ms without the patch, and ~1300ms with.

Here's how it will look after T215717 is fixed:


Here's the same graph without the intermediate step

dchan added a comment.Jul 13 2019, 2:38 PM

Monkey patched on live and edited a section of a long article (mobile site on desktop) (https://en.m.wikipedia.org/wiki/Megan_Rapinoe)
Testing second load times (i.e with the code and page data already fetched and cached), time to loaded was ~2300ms without the patch, and ~1300ms with.

Pretty exciting – thanks!

Change 522594 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Replace unrendered parts of the view tree with a stub node

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

Change 522935 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (213b79783)

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

Change 522935 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (213b79783)

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

Testing second load times (i.e with the code and page data already fetched and cached), time to loaded was ~2300ms without the patch, and ~1300ms with.

To clarify, these timings (2300 -> 1300ms) represent these sections in the visualisation (from the purple section onwards)

matmarex added a subscriber: matmarex.

Well, we did this, didn't we?

ppelberg closed this task as Resolved.Sep 3 2019, 12:27 AM
ppelberg added a comment.EditedSep 3 2019, 12:29 AM

Well, we did this, didn't we?

I remember us saying in chat on ~15 July this work was completed, so marking this task as Resolved. If this isn't the case, please re-open it. cc @Esanders