resourceloader: Optimise and simplify state propagation logic
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 8 Sep 2018 20:04:17 +0000 (21:04 +0100)
committerRoan Kattouw <roan.kattouw@gmail.com>
Thu, 13 Sep 2018 23:19:58 +0000 (16:19 -0700)
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

resources/src/startup/mediawiki.js

index ba8869b..c169487 100644 (file)
                         *    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.
                         *    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`:
                         * - `executing`:
                         *    The module is being executed.
                         * - `ready`:
                                /**
                                 * List of callback jobs waiting for modules to be ready.
                                 *
                                /**
                                 * 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.
                                 *
                                 * Typically when a job is created for a module, the job's dependencies contain
                                 * both the required module and all its recursive dependencies.
                                 *
                                 */
                                jobs = [],
 
                                 */
                                jobs = [],
 
+                               // For #requestPropagation() and #doPropagation()
+                               willPropagate = false,
+                               errorModules = [],
+
                                /**
                                 * @private
                                 * @property {Array} baseModules
                                /**
                                 * @private
                                 * @property {Array} baseModules
                        }
 
                        /**
                        }
 
                        /**
-                        * 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
                         *
                         * @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();
                                                                }
                                                                        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();
                                }
                        }
 
                                }
                        }
 
                                        if ( skip() ) {
                                                registry[ module ].skipped = true;
                                                registry[ module ].dependencies = [];
                                        if ( skip() ) {
                                                registry[ module ].skipped = true;
                                                registry[ module ].dependencies = [];
-                                               registry[ module ].state = 'ready';
-                                               handlePending( module );
+                                               setAndPropagate( module, 'ready' );
                                                return;
                                        }
                                }
                                                return;
                                        }
                                }
                                                // 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' ) {
                                                // 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 );
                                                        return;
                                                }
                                                queue.push( module );
                                        script = registry[ module ].script;
                                        markModuleReady = function () {
                                                $CODE.profileScriptEnd();
                                        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
                                        };
                                        nestedAddScript = function ( arr, callback, i ) {
                                                // Recursively call queueModuleScript() in its own callback
                                        } 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.
                                        } 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', {
                                                $CODE.profileScriptEnd();
                                                mw.trackError( 'resourceloader.exception', {
-                                                       exception: e, module:
-                                                       module, source: 'module-execute'
+                                                       exception: e,
+                                                       module: module,
+                                                       source: 'module-execute'
                                                } );
                                                } );
-                                               handlePending( module );
                                        }
                                };
 
                                        }
                                };
 
                                        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 ].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' );
                                        }
                                },
 
                                        }
                                },
 
                                                if ( !hasOwn.call( registry, module ) ) {
                                                        mw.loader.register( module );
                                                }
                                                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 );
                                        }
                                },
 
                                        }
                                },