resourceloader: Remove module stringification from execute path
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 31 Aug 2018 03:02:18 +0000 (04:02 +0100)
committerKrinkle <krinklemail@gmail.com>
Tue, 4 Sep 2018 17:55:25 +0000 (17:55 +0000)
After execute() is finished and markModuleReady() is called,
the execute path reaches handlePending() which calls
mw.loader.store.set(). This prepares the contents of the module
we just finished executing, for localStorage.

While the writing to localStorage was already debounced via #update,
the stringification of all functions and stylesheets was still
happening within the execute path, and other checks as well.

This commit replaces the mw.loader.store.set() call with a new
method mw.loader.store.add(). The serialisation is now performed
as part of flushWrites(), which is the debounced update that
happens in an idle callback.

While mw.loader.store is itself private within mw.loader, this
commit also marks the set() and update() methods as @private
within mw.loader.store as they are not (and should not) called
from outside this class.

Bug: T202703
Bug: T127328
Change-Id: I23ef28e096acf3bab57b88922ba21366fed06155

resources/src/startup/mediawiki.js
tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js

index f927aad..893e6d8 100644 (file)
                         *   - resolve: failed to sort dependencies for a module in mw.loader.load
                         *   - store-eval: could not evaluate module code cached in localStorage
                         *   - store-localstorage-init: localStorage or JSON parse error in mw.loader.store.init
-                        *   - store-localstorage-json: JSON conversion error in mw.loader.store.set
-                        *   - store-localstorage-update: localStorage or JSON conversion error in mw.loader.store.update
+                        *   - store-localstorage-json: JSON conversion error in mw.loader.store
+                        *   - store-localstorage-update: localStorage conversion error in mw.loader.store.
                         */
 
                        /**
 
                                // The current module became 'ready'.
                                if ( registry[ module ].state === 'ready' ) {
-                                       // Save it to the module store.
-                                       mw.loader.store.set( module, registry[ module ] );
+                                       // 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 ) {
                                        // to module implementations.
                                        items: {},
 
+                                       // Names of modules to be stored during the next update.
+                                       // See add() and update().
+                                       queue: [],
+
                                        // Cache hit stats
                                        stats: { hits: 0, misses: 0, expired: 0, failed: 0 },
 
                                        },
 
                                        /**
-                                        * Stringify a module and queue it for storage.
+                                        * Queue the name of a module that the next update should consider storing.
                                         *
+                                        * @since 1.32
                                         * @param {string} module Module name
-                                        * @param {Object} descriptor The module's descriptor as set in the registry
                                         */
-                                       set: function ( module, descriptor ) {
-                                               var args, key, src;
-
+                                       add: function ( module ) {
                                                if ( !this.enabled ) {
                                                        return;
                                                }
+                                               this.queue.push( module );
+                                               this.requestUpdate();
+                                       },
+
+                                       /**
+                                        * Add the contents of the named module to the in-memory store.
+                                        *
+                                        * This method does not guarantee that the module will be stored.
+                                        * Inspection of the module's meta data and size will ultimately decide that.
+                                        *
+                                        * This method is considered internal to mw.loader.store and must only
+                                        * be called if the store is enabled.
+                                        *
+                                        * @private
+                                        * @param {string} module Module name
+                                        */
+                                       set: function ( module ) {
+                                               var key, args, src,
+                                                       descriptor = mw.loader.moduleRegistry[ module ];
 
                                                key = getModuleKey( module );
 
                                                        // Already stored a copy of this exact version
                                                        key in this.items ||
                                                        // Module failed to load
+                                                       !descriptor ||
                                                        descriptor.state !== 'ready' ||
                                                        // Unversioned, private, or site-/user-specific
                                                        !descriptor.version ||
                                                        return;
                                                }
                                                this.items[ key ] = src;
-                                               this.update();
                                        },
 
                                        /**
                                        },
 
                                        /**
-                                        * Sync in-memory store back to localStorage.
+                                        * Request a sync of the in-memory store back to persisted localStorage.
                                         *
                                         * This function debounces updates. When called with a flush already pending, the
                                         * scheduled flush is postponed. The call to localStorage.setItem will be keep
                                         * is minor (merely a less efficient cache use) and the problem would be corrected
                                         * by subsequent page views.
                                         *
+                                        * This method is considered internal to mw.loader.store and must only
+                                        * be called if the store is enabled.
+                                        *
+                                        * @private
                                         * @method
                                         */
-                                       update: ( function () {
+                                       requestUpdate: ( function () {
                                                var timer, hasPendingWrites = false;
 
                                                function flushWrites() {
                                                        var data, key;
-                                                       if ( !mw.loader.store.enabled ) {
-                                                               return;
-                                                       }
 
+                                                       // Remove anything from the in-memory store that came from previous page
+                                                       // loads that no longer corresponds with current module names and versions.
                                                        mw.loader.store.prune();
+                                                       // Process queued module names, serialise their contents to the in-memory store.
+                                                       while ( mw.loader.store.queue.length ) {
+                                                               mw.loader.store.set( mw.loader.store.queue.shift() );
+                                                       }
+
                                                        key = mw.loader.store.getStoreKey();
                                                        try {
                                                                // Replacing the content of the module store might fail if the new
                                                }
 
                                                function request() {
-                                                       // If another mw.loader.store.set()/update() call happens in the narrow
+                                                       // If another mw.loader.store.requestUpdate() call happens in the narrow
                                                        // time window between requestIdleCallback() and flushWrites firing, ignore it.
                                                        // It'll be saved by the already-scheduled flush.
                                                        if ( !hasPendingWrites ) {
index 1a1affa..4cdf78e 100644 (file)
                mw.loader.register( 'test.stale', 'v2' );
                assert.strictEqual( mw.loader.store.get( 'test.stale' ), false, 'Not in store' );
 
+               // Stub deferred timeout/idle callback
+               this.sandbox.stub( window, 'setTimeout', function ( fn ) {
+                       fn();
+               } );
+               this.sandbox.stub( mw, 'requestIdleCallback', function ( fn ) {
+                       fn();
+               } );
+
                mw.loader.implement( 'test.stale@v1', function () {
                        count++;
                } );
                mw.loader.register( 'test.stalebc', 'v2' );
                assert.strictEqual( mw.loader.store.get( 'test.stalebc' ), false, 'Not in store' );
 
+               // Stub deferred timeout/idle callback
+               this.sandbox.stub( window, 'setTimeout', function ( fn ) {
+                       fn();
+               } );
+               this.sandbox.stub( mw, 'requestIdleCallback', function ( fn ) {
+                       fn();
+               } );
+
                mw.loader.implement( 'test.stalebc', function () {
                        script++;
                } );