Page MenuHomePhabricator

VisualEditor: Lazy-update SurfaceFragment
Closed, ResolvedPublic

Description

For performance reasons, we should lazy-update SurfaceFragments rather than updating them through a transact event handler.

The performance problem occurs when you create a bunch of fragments, then throw them away. The fragments are never actually destroyed, if only because the surface still has references to them because of the event binding. The discarded fragments continue to receive events about transactions and update themselves in response, but no one is using them any more.

Instead, I propose the following:

  • The surface should have an ordered array of [transaction, direction] pairs that tracks everything that has ever been applied. This is not the same as the undo/redo stacks: this particular array is append-only
  • Every SurfaceFragment has a property indicating which transaction (by index in the big array) is the last one that it knows about
  • SurfaceFragments do not listen for transact events
  • Instead, every time getRange() is called, the fragment asks the surface whether there are new transactions (with indices greater than the stored index), and if so it translates its range for them
    • For this to work, all internal uses of this.range need to be changed to this.getRange()

This'll make the fragment lazy-update its range, and we won't spend any time updating fragments that aren't being used.


Version: unspecified
Severity: normal

Details

Reference
bz47343

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:15 AM
bzimport set Reference to bz47343.

Trevor is adding a bunch of .destroy() calls to SurfaceFragment uses to mitigate the problem. These should be removed once this is implemented.

Related URL: https://gerrit.wikimedia.org/r/60256 (Gerrit Change I9e9818da1baa8319a3002f6d74fd1aad6732a8f5)