Page MenuHomePhabricator

Optimize some jQuery selectors
Closed, ResolvedPublic

Description

Optimizing some selectors in /modules/controller.js in DiscussionTools repository.

Replace:

$( '#ca-addsection a' )

with:

$( '#ca-addsection' ).find( 'a' )

Because as CSS selectors are evaluated right-to-left, that was analyzing each and every <a> of the page.

Also, replace:

$( '#t-permalink a, #coll-download-as-rl a' )

with:

$( '#t-permalink' ).add( '#coll-download-as-rl' ).find( 'a' )

Because this way jQuery uses two getElementById's, which are extremely fast.


Benchmark for 10,000 iterations (less is better):

$( '#t-permalink a, #coll-download-as-rl a' )
$( '#t-permalink, #coll-download-as-rl' ).find( 'a' )
$( '#t-permalink' ).add( '#coll-download-as-rl' ).find( 'a' )

3428 ms (which means it takes a third of a ms, just to execute this selector)
1042 ms 
146 ms

Two similar optimizations should be applied to /modules/ve-mw/init/targets/ve.init.mw.DesktopArticleTarget.js in VisualEditor repository.

Event Timeline

Change 864867 had a related patch set uploaded (by Gerrit Patch Uploader; author: Francois Pignon):

[mediawiki/extensions/DiscussionTools@master] Optimize some jQuery selectors

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

Change 864870 had a related patch set uploaded (by Gerrit Patch Uploader; author: Francois Pignon):

[mediawiki/extensions/VisualEditor@master] Optimize some jQuery selectors

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

Interesting, I knew that in the general case selectors are evaluated like that, but I expected the special case of an ID selector to be optimized somehow. It turns out that Firefox and Chrome indeed optimize $( '#ca-addsection a' ), but not $( '#t-permalink a, #coll-download-as-rl a' ) (and IE 11, which we still support, optimizes neither).

I don't think this will make a difference in perceived performance (the code in question is only evaluated once on page load and after saving changes), but it makes sense to me to apply the change just to promote it as a good practice.

Benchmark to test at home: visit https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical) and run the following in browser console:

var f = [
	function () { $( '#ca-addsection a' ) },
	function () { $( '#ca-addsection' ).find( 'a' ) },
	function () { $( '#t-permalink a, #coll-download-as-rl a' ) },
	function () { $( '#t-permalink, #coll-download-as-rl' ).find( 'a' ) },
	function () { $( '#t-permalink' ).add( '#coll-download-as-rl' ).find( 'a' ) },
];

for ( var i = 0; i < f.length; i++ ) {
	var ff = f[ i ];
	var t = Date.now();
	for ( var j = 0; j < 10000; j++ ) {
		ff();
	}
	console.log( i + ': ' + ( Date.now() - t ) + ' ms' );
}

Results on my computer:

FirefoxChromeInternet Explorer 11
$( '#ca-addsection a' )25 ms16 ms3385 ms
$( '#ca-addsection' ).find( 'a' )14 ms14 ms50 ms
$( '#t-permalink a, #coll-download-as-rl a' )2087 ms1727 ms4448 ms
$( '#t-permalink, #coll-download-as-rl' ).find( 'a' )832 ms568 ms2977 ms
$( '#t-permalink' ).add( '#coll-download-as-rl' ).find( 'a' )31 ms43 ms174 ms

Change 864867 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Optimize some jQuery selectors

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

Change 864870 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Optimize some jQuery selectors

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

I confirm Chrome and Firefox seem to have implemented some optimization for the $( '#ca-addsection a' ) case, because it's significantly faster than $( 'a' ), otherwise it would have been the contrary.

Though, I wouldn't rely on the fact that the browser has such optimization, as there are plenty other browsers in the wild. Your results with IE 11 clearly show the performance impact on an "unoptimized" browser.

Also, even on optimized browsers, $( '#ca-addsection' ).find( 'a' ) is still a bit faster than $( '#ca-addsection a' ), though both are very fast indeed.

matmarex assigned this task to Od1n.