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 ba2b684..620dd5e 100644 (file)
--- 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 9473137..53a714f 100644 (file)
--- 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 );