Index ResourceLoader dependency lists in startup
authorTrevor Parscal <trevorparscal@gmail.com>
Sat, 25 Oct 2014 00:18:24 +0000 (17:18 -0700)
committerRoan Kattouw <roan.kattouw@gmail.com>
Mon, 8 Dec 2014 19:04:41 +0000 (11:04 -0800)
By using the existing indexes of modules in the array being passed to
mw.loader.register we can reduce the size of the startup module by about
6% after gzip (nearly 20% before) on a wiki with very few modules (such
as my localhost). Comparing data from en.wikipedia.org shows about 9%
after gzip (nearly 30% before).

The technique adds a function to mediawiki.js which resolves the indexes
before registering the modules, which costs a little bit of data in that
payload, but it's negligible (118 bytes after gzip) in comparison to the
overall reduction.

Also, cleaned up lies in documentation and strange use of "m" as an
iterator variable.

Bonus: fix ISO8601 timestamp instead of UNIX timestamp being passed
to custom loader scripts.

Change-Id: If12991413fa6129cd20ceab0e59a3a30a4fdf5ce

includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderStartUpModule.php
resources/src/mediawiki/mediawiki.js
tests/phpunit/includes/resourceloader/ResourceLoaderStartupModuleTest.php

index 93cc245..8602fb0 100644 (file)
@@ -1219,6 +1219,24 @@ class ResourceLoader {
                $dependencies = null, $group = null, $source = null, $skip = null
        ) {
                if ( is_array( $name ) ) {
+                       // Build module name index
+                       $index = array();
+                       foreach ( $name as $i => $module ) {
+                               $index[$module[0]] = $i;
+                       }
+
+                       // Transform dependency names into indexes when possible, they will be resolved by
+                       // mw.loader.register on the other end
+                       foreach ( $name as &$module ) {
+                               if ( isset( $module[2] ) ) {
+                                       foreach ( $module[2] as &$dependency ) {
+                                               if ( isset( $index[$dependency] ) ) {
+                                                       $dependency = $index[$dependency];
+                                               }
+                                       }
+                               }
+                       }
+
                        return Xml::encodeJsCall(
                                'mw.loader.register',
                                array( $name ),
index 5370424..3b09059 100644 (file)
@@ -147,7 +147,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
        }
 
        /**
-        * Optimize the dependency tree in $this->modules and return it.
+        * Optimize the dependency tree in $this->modules.
         *
         * The optimization basically works like this:
         *      Given we have module A with the dependencies B and C
@@ -155,11 +155,11 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
         *      Now we don't have to tell the client to explicitly fetch module
         *              C as that's already included in module B.
         *
-        * This way we can reasonably reduce the amout of module registration
+        * This way we can reasonably reduce the amount of module registration
         * data send to the client.
         *
         * @param array &$registryData Modules keyed by name with properties:
-        *  - string 'version'
+        *  - number 'version'
         *  - array 'dependencies'
         *  - string|null 'group'
         *  - string 'source'
@@ -255,7 +255,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        if ( $data['loader'] !== false ) {
                                $out .= ResourceLoader::makeCustomLoaderScript(
                                        $name,
-                                       wfTimestamp( TS_ISO_8601_BASIC, $data['version'] ),
+                                       $data['version'],
                                        $data['dependencies'],
                                        $data['group'],
                                        $data['source'],
@@ -280,7 +280,11 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        ) {
                                // Modules with dependencies but no group, foreign source or skip function;
                                // call mw.loader.register(name, timestamp, dependencies)
-                               $registrations[] = array( $name, $data['version'], $data['dependencies'] );
+                               $registrations[] = array(
+                                       $name,
+                                       $data['version'],
+                                       $data['dependencies']
+                               );
                        } elseif (
                                $data['source'] === 'local' &&
                                $data['skip'] === null
index 06d0f47..bb91663 100644 (file)
                                addScript( sourceLoadScript + '?' + $.param( request ) + '&*', null, async );
                        }
 
+                       /**
+                        * Resolve indexed dependencies.
+                        *
+                        * ResourceLoader uses an optimization to save space which replaces module names in
+                        * dependency lists with the index of that module within the array of module
+                        * registration data if it exists. The benefit is a significant reduction in the data
+                        * size of the startup module. This function changes those dependency lists back to
+                        * arrays of strings.
+                        *
+                        * @param {Array} modules Modules array
+                        */
+                       function resolveIndexedDependencies( modules ) {
+                               var i, iLen, j, jLen, module, dependency;
+
+                               // Expand indexed dependency names
+                               for ( i = 0, iLen = modules.length; i < iLen; i++ ) {
+                                       module = modules[i];
+                                       if ( module[2] ) {
+                                               for ( j = 0, jLen = module[2].length; j < jLen; j++ ) {
+                                                       dependency = module[2][j];
+                                                       if ( typeof dependency === 'number' ) {
+                                                               module[2][j] = modules[dependency][0];
+                                                       }
+                                               }
+                                       }
+                               }
+                       }
+
                        /* Public Members */
                        return {
                                /**
                                 * Register a module, letting the system know about it and its
                                 * properties. Startup modules contain calls to this function.
                                 *
-                                * @param {string} module Module name
+                                * When using multiple module registration by passing an array, dependencies that
+                                * are specified as references to modules within the array will be resolved before
+                                * the modules are registered.
+                                *
+                                * @param {string|Array} module Module name or array of arrays, each containing
+                                *   a list of arguments compatible with this method
                                 * @param {number} version Module version number as a timestamp (falls backs to 0)
                                 * @param {string|Array|Function} dependencies One string or array of strings of module
                                 *  names on which this module depends, or a function that returns that array.
                                 * @param {string} [skip=null] Script body of the skip function
                                 */
                                register: function ( module, version, dependencies, group, source, skip ) {
-                                       var m;
+                                       var i, len;
                                        // Allow multiple registration
                                        if ( typeof module === 'object' ) {
-                                               for ( m = 0; m < module.length; m += 1 ) {
+                                               resolveIndexedDependencies( module );
+                                               for ( i = 0, len = module.length; i < len; i++ ) {
                                                        // module is an array of module names
-                                                       if ( typeof module[m] === 'string' ) {
-                                                               mw.loader.register( module[m] );
+                                                       if ( typeof module[i] === 'string' ) {
+                                                               mw.loader.register( module[i] );
                                                        // module is an array of arrays
-                                                       } else if ( typeof module[m] === 'object' ) {
-                                                               mw.loader.register.apply( mw.loader, module[m] );
+                                                       } else if ( typeof module[i] === 'object' ) {
+                                                               mw.loader.register.apply( mw.loader, module[i] );
                                                        }
                                                }
                                                return;
index 3af1d1f..6bfa40f 100644 (file)
@@ -115,8 +115,8 @@ mw.loader.addSource( {
                                        'test.z.foo' => new ResourceLoaderTestModule( array(
                                                'dependencies' => array(
                                                        'test.x.core',
-                                                       'test.x.polyfil',
-                                                       'test.y.polyfil',
+                                                       'test.x.polyfill',
+                                                       'test.y.polyfill',
                                                ),
                                        ) ),
                                ),
@@ -148,9 +148,9 @@ mw.loader.addSource( {
         "test.z.foo",
         1388534400,
         [
-            "test.x.core",
-            "test.x.polyfil",
-            "test.y.polyfil"
+            0,
+            1,
+            2
         ]
     ]
 ] );',
@@ -232,29 +232,29 @@ mw.loader.addSource( {
         "test.x.util",
         1388534400,
         [
-            "test.x.core"
+            1
         ]
     ],
     [
         "test.x.foo",
         1388534400,
         [
-            "test.x.core"
+            1
         ]
     ],
     [
         "test.x.bar",
         1388534400,
         [
-            "test.x.util"
+            2
         ]
     ],
     [
         "test.x.quux",
         1388534400,
         [
-            "test.x.foo",
-            "test.x.bar",
+            3,
+            4,
             "test.x.unknown"
         ]
     ],
@@ -345,7 +345,7 @@ mw.loader.addSource( {
 'mw.loader.addSource({"local":"/w/load.php"});'
 . 'mw.loader.register(['
 . '["test.blank",1388534400],'
-. '["test.min",1388534400,["test.blank"],null,"local",'
+. '["test.min",1388534400,[0],null,"local",'
 . '"return!!(window.JSON\u0026\u0026JSON.parse\u0026\u0026JSON.stringify);"'
 . ']]);',
                        $module->getModuleRegistrations( $context ),
@@ -373,7 +373,7 @@ mw.loader.addSource( {
         "test.min",
         1388534400,
         [
-            "test.blank"
+            0
         ],
         null,
         "local",