From: Timo Tijhof Date: Sat, 8 Sep 2018 20:04:17 +0000 (+0100) Subject: resourceloader: Optimise and simplify state propagation logic X-Git-Tag: 1.34.0-rc.0~4094^2 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=6a83163b1cbad14e988ec0fbc7aede50290ae13b resourceloader: Optimise and simplify state propagation logic aka "handlePending 2.0", aka "don't recurse 300x before executing a module", aka "don't break DevTools flame graphs". == Impact Comparison based on viewing the default Main page on Chrome stable. The local MediaWiki has a few extensions installed (EventLogging, ULS, Navigation Timing). Measured by alternating between before/after and logging 'mediaWikiLoadEnd' from ext.navigationTiming.js, and evaluating the following on the console: ``` ({ responseStart: performance.timing.responseStart - performance.timing.navigationStart, domInteractive: performance.timing.domInteractive - performance.timing.navigationStart, domComplete: performance.timing.domComplete - performance.timing.navigationStart, loadEventEnd: performance.timing.loadEventEnd - performance.timing.navigationStart }); ``` This was repeated five times, and I picked three results based on similar responseStart values. This provides a fairer comparison by avoiding bias of fluctuation from the network/server. The actual values ended up slightly favouring the older code. | -------------- | ---------------- | ---------------- | -------- | | | Before | After | Avg diff | | -------------- | ---------------- | ---------------- | -------- | | responseStart | 1044, 1001, 1016 | 1025, 1023, 1024 | +3ms | | domInteractive | 2080, 2069, 2059 | 1872, 2101, 2050 | -61ms | | domComplete | 4361, 4239, 3927 | 3691, 4023, 3981 | -227ms | | loadEventEnd | 4366, 4244, 3932 | 3691, 4023, 3982 | -282ms | | mwLoadEnd | 4524, 4416, 4113 | 3994, 4320, 4297 | -147ms | | -------------- | ---------------- | ---------------- | -------- | == Implementation While technically a single logical change, this commit does resolve multiple long-standing issues and inefficiencies. * handlePending (now called doPropagation) was called way more often than needed. When a load.php response arrived with calls to mw.loader.implement(), each one could execute and immediately call handlePending(). Now, the first implementation in a batch schedule one call doPropagation(), and the later ones ride along that one call. * Most calls to handlePending were only able to execute one pending module, which in turn created its own handlePending call that started all over again. This meant that by the time control returned to an outer call, there was nothing left to do, except it still needed to continue its iteration over the whole registry before knowing there was nothing left to dos. * Due to recursive calls to handlePending() from execute(), and due to immediate execution from implement() - as called from load.php or asyncEval - the stack was often already 100s of level deep before *starting* the execution of a module. Such deep stacks caused: - a larger memory footprint (for the stacks themselves). - confusing flame graphs. It was impossible to analyze performance of module initialisation, I typically could only look at code from dom-ready handlers or other events. The stacks were so big, they actually caused rendering bugs inside Chrome where higher parts of the stack would be trimmed, and thus related code would no longer be visually grouped by the same parent. - confusing error messages (long stack traces). * The eager execution from mw.loader.implement() calls meant that it was not possible to separate the parsing/loading of code (by load.php and asyncEval), from the execution of code. Now, this is separated by a 1ms yield (in practice it's larger than 1ms, but it has high priority). This means that the batch of localStorage eval() and the batch response from load.php can now first do one type of work (allocating of functions and objects, stashing them in the registry), and then later the other type of work (execution of the module code) - with some breathing room allowed for rendering. Bug: T127328 Bug: T202703 Change-Id: I499ae3f095545abcc03e8989f54422b1997738d3 --- diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js index ba8869bd28..c16948703b 100644 --- a/resources/src/startup/mediawiki.js +++ b/resources/src/startup/mediawiki.js @@ -569,8 +569,8 @@ * The contents are then stashed in the registry via mw.loader#implement. * - `loaded`: * The module has been loaded from the server and stashed via mw.loader#implement. - * If the module has no more dependencies in-flight, the module will be executed - * immediately. Otherwise execution is deferred, controlled via #handlePending. + * Once the module has no more dependencies in-flight, the module will be executed, + * controlled via #requestPropagation and #doPropagation. * - `executing`: * The module is being executed. * - `ready`: @@ -605,8 +605,7 @@ /** * List of callback jobs waiting for modules to be ready. * - * Jobs are created by #enqueue() and run by #handlePending(). - * + * Jobs are created by #enqueue() and run by #doPropagation(). * Typically when a job is created for a module, the job's dependencies contain * both the required module and all its recursive dependencies. * @@ -623,6 +622,10 @@ */ jobs = [], + // For #requestPropagation() and #doPropagation() + willPropagate = false, + errorModules = [], + /** * @private * @property {Array} baseModules @@ -781,86 +784,133 @@ } /** - * A module has entered state 'ready', 'error', or 'missing'. Automatically update - * pending jobs and modules that depend upon this module. If the given module failed, - * propagate the 'error' state up the dependency tree. Otherwise, go ahead and execute - * all jobs/modules now having their dependencies satisfied. + * Handle propagation of module state changes and reactions to them. * - * Jobs that depend on a failed module, will have their error callback ran (if any). + * - When a module reaches a failure state, this should be propagated to + * modules that depend on the failed module. + * - When a module reaches a final state, pending job callbacks for the + * module from mw.loader.using() should be called. + * - When a module reaches the 'ready' state from #execute(), consider + * executing dependant modules now having their dependencies satisfied. + * - When a module reaches the 'loaded' state from mw.loader.implement, + * consider executing it, if it has no unsatisfied dependencies. * * @private - * @param {string} module Name of module that entered one of the states 'ready', 'error', or 'missing'. */ - function handlePending( module ) { - var j, job, hasErrors, m, stateChange, fromBaseModule; - - if ( registry[ module ].state === 'error' || registry[ module ].state === 'missing' ) { - fromBaseModule = baseModules.indexOf( module ) !== -1; - // If the current module failed, mark all dependent modules also as failed. - // Iterate until steady-state to propagate the error state upwards in the - // dependency tree. - do { - stateChange = false; - for ( m in registry ) { - if ( registry[ m ].state !== 'error' && registry[ m ].state !== 'missing' ) { - // Always propagate errors from base modules to regular modules (implicit dependency). - // Between base modules or regular modules, consider direct dependencies only. - if ( - ( fromBaseModule && baseModules.indexOf( m ) === -1 ) || - anyFailed( registry[ m ].dependencies ) - ) { - registry[ m ].state = 'error'; - stateChange = true; + function doPropagation() { + var errorModule, baseModuleError, module, i, failed, job, + didPropagate = true; + + // Keep going until the last iteration performed no actions. + do { + didPropagate = false; + + // Stage 1: Propagate failures + while ( errorModules.length ) { + errorModule = errorModules.shift(); + baseModuleError = baseModules.indexOf( errorModule ) !== -1; + for ( module in registry ) { + if ( registry[ module ].state !== 'error' && registry[ module ].state !== 'missing' ) { + if ( baseModuleError && baseModules.indexOf( module ) === -1 ) { + // Propate error from base module to all regular (non-base) modules + registry[ module ].state = 'error'; + didPropagate = true; + } else if ( registry[ module ].dependencies.indexOf( errorModule ) !== -1 ) { + // Propagate error from dependency to depending module + registry[ module ].state = 'error'; + // .. and propagate it further + errorModules.push( module ); + didPropagate = true; } } } - } while ( stateChange ); - } + } - // Execute all jobs whose dependencies are either all satisfied or contain at least one failed module. - for ( j = 0; j < jobs.length; j++ ) { - hasErrors = anyFailed( jobs[ j ].dependencies ); - if ( hasErrors || allReady( jobs[ j ].dependencies ) ) { - // All dependencies satisfied, or some have errors - job = jobs[ j ]; - jobs.splice( j, 1 ); - j -= 1; - try { - if ( hasErrors ) { - if ( typeof job.error === 'function' ) { - job.error( new Error( 'Module ' + module + ' has failed dependencies' ), [ module ] ); - } - } else { - if ( typeof job.ready === 'function' ) { + // Stage 2: Execute 'loaded' modules with no unsatisfied dependencies + for ( module in registry ) { + if ( registry[ module ].state === 'loaded' && allWithImplicitReady( module ) ) { + // Recursively execute all dependent modules that were already loaded + // (waiting for execution) and no longer have unsatisfied dependencies. + // Base modules may have dependencies amongst eachother to ensure correct + // execution order. Regular modules wait for all base modules. + // eslint-disable-next-line no-use-before-define + execute( module ); + didPropagate = true; + } + } + + // Stage 3: Invoke job callbacks that are no longer blocked + for ( i = 0; i < jobs.length; i++ ) { + job = jobs[ i ]; + failed = anyFailed( job.dependencies ); + if ( failed || allReady( job.dependencies ) ) { + jobs.splice( i, 1 ); + i -= 1; + try { + if ( failed && job.error ) { + job.error( new Error( 'Module has failed dependencies' ), job.dependencies ); + } else if ( !failed && job.ready ) { job.ready(); } + } catch ( e ) { + // A user-defined callback raised an exception. + // Swallow it to protect our state machine! + mw.trackError( 'resourceloader.exception', { + exception: e, + source: 'load-callback' + } ); } - } catch ( e ) { - // A user-defined callback raised an exception. - // Swallow it to protect our state machine! - mw.trackError( 'resourceloader.exception', { - exception: e, - module: module, - source: 'load-callback' - } ); + didPropagate = true; } } + } while ( didPropagate ); + + willPropagate = false; + } + + /** + * Request a (debounced) call to doPropagation(). + * + * @private + */ + function requestPropagation() { + if ( willPropagate ) { + // Already scheduled, or, we're already in a doPropagation stack. + return; } + willPropagate = true; + // Yield for two reasons: + // * Allow successive calls to mw.loader.implement() from the same + // load.php response, or from the same asyncEval() to be in the + // propagation batch. + // * Allow the browser to breathe between the reception of + // module source code and the execution of it. + // + // Use a high priority because the user may be waiting for interactions + // to start being possible. But, first provide a moment (up to 'timeout') + // for native input event handling (e.g. scrolling/typing/clicking). + mw.requestIdleCallback( doPropagation, { timeout: 1 } ); + } - // The current module became 'ready'. - if ( registry[ module ].state === 'ready' ) { - // Queue it for later syncing to the module store. - mw.loader.store.add( module ); - // Recursively execute all dependent modules that were already loaded - // (waiting for execution) and no longer have unsatisfied dependencies. - for ( m in registry ) { - // Base modules may have dependencies amongst eachother to ensure correct - // execution order. Regular modules wait for all base modules. - if ( registry[ m ].state === 'loaded' && allWithImplicitReady( m ) ) { - // eslint-disable-next-line no-use-before-define - execute( m ); - } + /** + * Update a module's state in the registry and make sure any neccesary + * propagation will occur. See #doPropagation for more about propagation. + * See #registry for more about how states are used. + * + * @private + * @param {string} module + * @param {string} state + */ + function setAndPropagate( module, state ) { + registry[ module ].state = state; + if ( state === 'loaded' || state === 'ready' || state === 'error' || state === 'missing' ) { + if ( state === 'ready' ) { + // Queue to later be synced to the local module store. + mw.loader.store.add( module ); + } else if ( state === 'error' || state === 'missing' ) { + errorModules.push( module ); } + requestPropagation(); } } @@ -892,8 +942,7 @@ if ( skip() ) { registry[ module ].skipped = true; registry[ module ].dependencies = []; - registry[ module ].state = 'ready'; - handlePending( module ); + setAndPropagate( module, 'ready' ); return; } } @@ -1127,8 +1176,7 @@ // Private modules must be embedded in the page. Don't bother queuing // these as the server will deny them anyway (T101806). if ( registry[ module ].group === 'private' ) { - registry[ module ].state = 'error'; - handlePending( module ); + setAndPropagate( module, 'error' ); return; } queue.push( module ); @@ -1165,8 +1213,7 @@ script = registry[ module ].script; markModuleReady = function () { $CODE.profileScriptEnd(); - registry[ module ].state = 'ready'; - handlePending( module ); + setAndPropagate( module, 'ready' ); }; nestedAddScript = function ( arr, callback, i ) { // Recursively call queueModuleScript() in its own callback @@ -1216,13 +1263,13 @@ } catch ( e ) { // Use mw.track instead of mw.log because these errors are common in production mode // (e.g. undefined variable), and mw.log is only enabled in debug mode. - registry[ module ].state = 'error'; + setAndPropagate( module, 'error' ); $CODE.profileScriptEnd(); mw.trackError( 'resourceloader.exception', { - exception: e, module: - module, source: 'module-execute' + exception: e, + module: module, + source: 'module-execute' } ); - handlePending( module ); } }; @@ -1855,10 +1902,7 @@ registry[ name ].templates = templates || null; // The module may already have been marked as erroneous if ( registry[ name ].state !== 'error' && registry[ name ].state !== 'missing' ) { - registry[ name ].state = 'loaded'; - if ( allWithImplicitReady( name ) ) { - execute( name ); - } + setAndPropagate( name, 'loaded' ); } }, @@ -1929,12 +1973,7 @@ if ( !hasOwn.call( registry, module ) ) { mw.loader.register( module ); } - registry[ module ].state = state; - if ( state === 'ready' || state === 'error' || state === 'missing' ) { - // Make sure pending modules depending on this one get executed if their - // dependencies are now fulfilled! - handlePending( module ); - } + setAndPropagate( module, state ); } },