From c42bc78530771a7e16433fdf6cfe3b73ed5626b6 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 25 Aug 2016 16:08:56 -0700 Subject: [PATCH] mw.loader: Minor clean up and optimisations * mw.loader.state: Simplify code by removing redundant branch that could set the value to what it already was. Also remove the condition for it since re-setting it should never happen in practice. If it does happen, it's a harmless no-op. * mw.loader.store.set: Document use cases for descriptor being partial. On a most page views, this branch runs for 'site.styles', 'json', 'user', 'dom-level2-shim', 'mediawiki.hidpi', 'user.options', and 'user.tokens'. Aside from group=user modules, the others are skipped or style modules. * register: Make faster and reduce GC overhead (MacBookPro, Chrome canary 52, Vagrant, 566 registered modules) Measured using Timeline (JS Profile) and Profile (Allocation Profile). - Use typeof instead of isFunction(). - In resolveIndexedDependencies: - Use a for-loop instead of $.each(). This uncovered a JSHint warning for making $.map() functions in a loop. Move the function out and re-use it. - Use a for-loop instead of $.map(). - Don't create an empty dependencies array in the registry only to dereference it in the next statement. Time for register() down from 5-10ms to 3-4ms. Memory for register() down from 153KB (88% in register, 12% in resolveIndexedDependencies/each/map) to 120KB (100% in register, no mention of resolveIndexedDependencies) GC down from 350KB to 240KB. * Remove use of isFunction(). We're not interested in its edge cases of native functions in old IE and regex false-positive in Android 2.3. For plain user-specified functions, typeof will return 'function' in all supported browsers. Change-Id: I43fc7c7f54d0c279b659d6fddd21673de48f4bc2 --- resources/src/mediawiki/mediawiki.js | 52 +++++++++++++++------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index 491564ade7..af37162cbe 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -1068,11 +1068,11 @@ j -= 1; try { if ( hasErrors ) { - if ( $.isFunction( job.error ) ) { + if ( typeof job.error === 'function' ) { job.error( new Error( 'Module ' + module + ' has failed dependencies' ), [ module ] ); } } else { - if ( $.isFunction( job.ready ) ) { + if ( typeof job.ready === 'function' ) { job.ready(); } } @@ -1131,7 +1131,7 @@ } // Resolves dynamic loader function and replaces it with its own results - if ( $.isFunction( registry[ module ].dependencies ) ) { + if ( typeof registry[ module ].dependencies === 'function' ) { registry[ module ].dependencies = registry[ module ].dependencies(); // Ensures the module's dependencies are always in an array if ( typeof registry[ module ].dependencies !== 'object' ) { @@ -1309,7 +1309,7 @@ legacyWait.always( function () { if ( $.isArray( script ) ) { nestedAddScript( script, markModuleReady, 0 ); - } else if ( $.isFunction( script ) ) { + } else if ( typeof script === 'function' ) { // Pass jQuery twice so that the signature of the closure which wraps // the script can bind both '$' and 'jQuery'. script( $, $, mw.loader.require, registry[ module ].module ); @@ -1547,13 +1547,18 @@ * @param {Array} modules Modules array */ function resolveIndexedDependencies( modules ) { - $.each( modules, function ( idx, module ) { - if ( module[ 2 ] ) { - module[ 2 ] = $.map( module[ 2 ], function ( dep ) { - return typeof dep === 'number' ? modules[ dep ][ 0 ] : dep; - } ); + var i, j, deps; + function resolveIndex( dep ) { + return typeof dep === 'number' ? modules[ dep ][ 0 ] : dep; + } + for ( i = 0; i < modules.length; i++ ) { + deps = modules[ i ][ 2 ]; + if ( deps ) { + for ( j = 0; j < deps.length; j++ ) { + deps[ j ] = resolveIndex( deps[ j ] ); + } } - } ); + } } /** @@ -1798,7 +1803,7 @@ * @param {string} [skip=null] Script body of the skip function */ register: function ( module, version, dependencies, group, source, skip ) { - var i; + var i, deps; // Allow multiple registration if ( typeof module === 'object' ) { resolveIndexedDependencies( module ); @@ -1816,6 +1821,13 @@ if ( hasOwn.call( registry, module ) ) { throw new Error( 'module already registered: ' + module ); } + if ( typeof dependencies === 'string' ) { + // A single module name + deps = [ dependencies ]; + } else if ( typeof dependencies === 'object' || typeof dependencies === 'function' ) { + // Array of module names or a function that returns an array + deps = dependencies; + } // List the module as registered registry[ module ] = { // Exposed to execute() for mw.loader.implement() closures. @@ -1824,20 +1836,12 @@ exports: {} }, version: version !== undefined ? String( version ) : '', - dependencies: [], + dependencies: deps || [], group: typeof group === 'string' ? group : null, source: typeof source === 'string' ? source : 'local', state: 'registered', skip: typeof skip === 'string' ? skip : null }; - if ( typeof dependencies === 'string' ) { - // Allow dependencies to be given as a single module name - registry[ module ].dependencies = [ dependencies ]; - } else if ( typeof dependencies === 'object' || $.isFunction( dependencies ) ) { - // Allow dependencies to be given as an array of module names - // or a function which returns an array - registry[ module ].dependencies = dependencies; - } }, /** @@ -2023,14 +2027,11 @@ if ( !hasOwn.call( registry, module ) ) { mw.loader.register( module ); } - if ( $.inArray( state, [ 'ready', 'error', 'missing' ] ) !== -1 - && registry[ module ].state !== state ) { + registry[ module ].state = state; + if ( $.inArray( state, [ 'ready', 'error', 'missing' ] ) !== -1 ) { // Make sure pending modules depending on this one get executed if their // dependencies are now fulfilled! - registry[ module ].state = state; handlePending( module ); - } else { - registry[ module ].state = state; } }, @@ -2275,6 +2276,7 @@ // Unversioned, private, or site-/user-specific ( !descriptor.version || $.inArray( descriptor.group, [ 'private', 'user' ] ) !== -1 ) || // Partial descriptor + // (e.g. skipped module, or style module with state=ready) $.inArray( undefined, [ descriptor.script, descriptor.style, descriptor.messages, descriptor.templates ] ) !== -1 ) { -- 2.20.1