Page MenuHomePhabricator

VisualEditor: ve.ce.ProtectedNode.prototype.onProtectedSetup is a performance hog
Closed, ResolvedPublic

Description

Running JavaScript profiling on both desktop and mobile the majority of time is spent in jQuery.extend.css - the majority of which seems to be caused by ve.ce.ProtectedNode.prototype.onProtectedSetup

On mobile this seems to be even more severe.

In desktop 7.9% of execution time to load VisualEditor was spent in jQuery.extend.css - this was the most heavy function. In mobile it was also the most heavy - however in mobile a whopping 23.76% of time was spent in jQuery.extend.css

Stubbing out the contents of the foreach loop
this.$element.add( this.$element.find( '*' ) ).each( function () {

seems to remedy this problem.


Version: unspecified
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=51202

Details

Reference
bz64709

Event Timeline

The majority of time is spent in looking up the float property but importNode is also a relatively big performance hog here

There may be ways to improve this in the short term, but what we really need to do is experiment with using an SVG overlay.

jgonera wrote:

The question is, why is this more of a problem with mobile skin than with Vector?

We have a lot more DOM nodes in mobile - 2 times the number of DOM nodes in an article. In desktop they replace the page rather than create an overlay..

So I tried removing #content element - no visible difference.
Killing our stylesheets - visible difference (drops 1.59%).

This must be linked to causing reflows, that are more expensive in the Minerva skin.

I went deeper down the rabbit hole...

Removing all button styling drops this by 23.76% to 16.4%
It drops to 8.6% with removal of 'skins.minerva.chrome.styles', 8.6% when I drop the use of ui.less

Both the stylings for buttons and ui.less use last-child, nth-child, first-child, not selectors. These seem to be the major pain point.

See bug 64719.

https://gerrit.wikimedia.org/r/130823 (for bug 52499) should make this perform a bit better as well, by 1) reducing the number of elements protectedNode CSS is applied to and 2) targeting them with a class rather than a star selector: .ve-ce-protectedNode * is an expensive selector just like .header > *:nth-child(3)

Additional fixes made for the 2014-05-15 release – but not sure what counts as "fixing" this bug. Jon, could you re-test?

Based on the work that Ed did, I'm going to mark this as closed.