From: Ori Livneh <ori@wikimedia.org>
Date: Mon, 23 Nov 2015 21:59:21 +0000 (-0800)
Subject: Make $.fn.updateTooltipAccessKeys() less expensive
X-Git-Tag: 1.31.0-rc.0~8896^2
X-Git-Url: http://git.cyclocoop.org/%24self?a=commitdiff_plain;h=9f581191d20c796bb449151c4643ee941bf8927a;p=lhc%2Fweb%2Fwiklou.git

Make $.fn.updateTooltipAccessKeys() less expensive

On page.ready on desktop skins, we call $.fn.updateTooltipAccessKeys(), which
updates about 20 elements (depending on the page and skin). Each invocation
parser the 'brackets' message, which ends up being quite expensive. We
previously had some caching to mitigate the cost, but it was removed in
I955ee61f6.

We can re-introduce caching without re-introducing the bug that I955ee61f6
sought to mitigate by making the astCache property unique to each parser
instance, as suggested in T54042#545717, and by ensuring the message function
is cached.

Change-Id: Ia1b42c70d8fc6ce03c8708f03c2b835942c4ead3
---

diff --git a/resources/src/mediawiki/mediawiki.jqueryMsg.js b/resources/src/mediawiki/mediawiki.jqueryMsg.js
index ba2b6849c7..620dd5e5af 100644
--- a/resources/src/mediawiki/mediawiki.jqueryMsg.js
+++ b/resources/src/mediawiki/mediawiki.jqueryMsg.js
@@ -132,10 +132,9 @@
 	 * @return {jQuery} return.return
 	 */
 	function getFailableParserFn( options ) {
-		var parser = new mw.jqueryMsg.parser( options );
-
 		return function ( args ) {
 			var fallback,
+				parser = new mw.jqueryMsg.parser( options ),
 				key = args[ 0 ],
 				argsArray = $.isArray( args[ 1 ] ) ? args[ 1 ] : slice.call( args, 1 );
 			try {
@@ -265,6 +264,7 @@
 	mw.jqueryMsg.parser = function ( options ) {
 		this.settings = $.extend( {}, parserDefaults, options );
 		this.settings.onlyCurlyBraceTransform = ( this.settings.format === 'text' || this.settings.format === 'escaped' );
+		this.astCache = {};
 
 		this.emitter = new mw.jqueryMsg.htmlEmitter( this.settings.language, this.settings.magic );
 	};
@@ -280,7 +280,8 @@
 		 * @return {jQuery}
 		 */
 		parse: function ( key, replacements ) {
-			return this.emitter.emit( this.getAst( key ), replacements );
+			var ast = this.getAst( key );
+			return this.emitter.emit( ast, replacements );
 		},
 
 		/**
@@ -291,11 +292,16 @@
 		 * @return {string|Array} string of '[key]' if message missing, simple string if possible, array of arrays if needs parsing
 		 */
 		getAst: function ( key ) {
-			var wikiText = this.settings.messages.get( key );
-			if ( typeof wikiText !== 'string' ) {
-				wikiText = '\\[' + key + '\\]';
+			var wikiText;
+
+			if ( !this.astCache.hasOwnProperty( key ) ) {
+				wikiText = this.settings.messages.get( key );
+				if ( typeof wikiText !== 'string' ) {
+					wikiText = '\\[' + key + '\\]';
+				}
+				this.astCache[ key ] = this.wikiTextToAst( wikiText );
 			}
-			return this.wikiTextToAst( wikiText );
+			return this.astCache[ key ];
 		},
 
 		/**
@@ -1260,23 +1266,19 @@
 	// Replace the default message parser with jqueryMsg
 	oldParser = mw.Message.prototype.parser;
 	mw.Message.prototype.parser = function () {
-		var messageFunction;
-
-		// TODO: should we cache the message function so we don't create a new one every time? Benchmark this maybe?
-		// Caching is somewhat problematic, because we do need different message functions for different maps, so
-		// we'd have to cache the parser as a member of this.map, which sounds a bit ugly.
-		// Do not use mw.jqueryMsg unless required
 		if ( this.format === 'plain' || !/\{\{|[\[<>&]/.test( this.map.get( this.key ) ) ) {
 			// Fall back to mw.msg's simple parser
 			return oldParser.apply( this );
 		}
 
-		messageFunction = mw.jqueryMsg.getMessageFunction( {
-			messages: this.map,
-			// For format 'escaped', escaping part is handled by mediawiki.js
-			format: this.format
-		} );
-		return messageFunction( this.key, this.parameters );
+		if ( !this.map.hasOwnProperty( this.format ) ) {
+			this.map[ this.format ] = mw.jqueryMsg.getMessageFunction( {
+				messages: this.map,
+				// For format 'escaped', escaping part is handled by mediawiki.js
+				format: this.format
+			} );
+		}
+		return this.map[ this.format ]( this.key, this.parameters );
 	};
 
 	/**
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js
index 94731377f5..53a714f81c 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js
@@ -739,6 +739,7 @@
 			message[ format ]();
 			assert.strictEqual( outerCalled, shouldCall, 'Outer function called for ' + key );
 			assert.strictEqual( innerCalled, shouldCall, 'Inner function called for ' + key );
+			delete mw.messages[ format ];
 		}
 
 		verifyGetMessageFunction( 'curly-brace', 'parse', true );