resourceloader: Reduce memory cost of mw.config.set()
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 31 Aug 2018 01:32:18 +0000 (02:32 +0100)
committerNikerabbit <niklas.laxstrom@gmail.com>
Fri, 31 Aug 2018 07:38:29 +0000 (07:38 +0000)
When capturing an Allocation Timeline in Chrome DevTools' Memory
panel, I noticed that there are a *ton* of strings and StringSet
memory allocations happening in ways I did not anticipate would
happen as part of calling  mw.config.set().

Given that legacy globals are currently enabled on WMF wikis,
this means that when calling mw.config.set() in the startup
module, the page header, and page footer (typically about ~200
keys in total), it also calls mw.log.deprecate() to create the
global alias.

And mw.log.deprecate() was allocating the StringSet and log
messages when creating the property, instead of only if and when
a 'get' or 'set' is triggered, which itself should be rare given
that the aliases are deprecated. So even when they are never called,
they were still creating a lot of objects that aren't used.

This commit instead creates the StringSet object lazily.
Also, given that logging is deduplicated, create the log message
only where we use it (once), instead of storing in the outer
closure that persists.

Bug: T127328
Change-Id: I7a16277f743ff39d81f8746dd2147ed8797c1c7a

resources/src/startup/mediawiki.js

index 3fb7ffc..cc02283 100644 (file)
                log.deprecate = !Object.defineProperty ? function ( obj, key, val ) {
                        obj[ key ] = val;
                } : function ( obj, key, val, msg, logName ) {
-                       var logged = new StringSet();
-                       logName = logName || key;
-                       msg = 'Use of "' + logName + '" is deprecated.' + ( msg ? ( ' ' + msg ) : '' );
-                       function uniqueTrace() {
-                               var trace = new Error().stack;
-                               if ( logged.has( trace ) ) {
-                                       return false;
+                       var stacks;
+                       function maybeLog() {
+                               var name,
+                                       trace = new Error().stack;
+                               if ( !stacks ) {
+                                       stacks = new StringSet();
+                               }
+                               if ( !stacks.has( trace ) ) {
+                                       stacks.add( trace );
+                                       name = logName || key;
+                                       mw.track( 'mw.deprecate', name );
+                                       mw.log.warn(
+                                               'Use of "' + name + '" is deprecated.' + ( msg ? ( ' ' + msg ) : '' )
+                                       );
                                }
-                               logged.add( trace );
-                               return true;
                        }
                        // Support: Safari 5.0
                        // Throws "not supported on DOM Objects" for Node or Element objects (incl. document)
                                        configurable: true,
                                        enumerable: true,
                                        get: function () {
-                                               if ( uniqueTrace() ) {
-                                                       mw.track( 'mw.deprecate', logName );
-                                                       mw.log.warn( msg );
-                                               }
+                                               maybeLog();
                                                return val;
                                        },
                                        set: function ( newVal ) {
-                                               if ( uniqueTrace() ) {
-                                                       mw.track( 'mw.deprecate', logName );
-                                                       mw.log.warn( msg );
-                                               }
+                                               maybeLog();
                                                val = newVal;
                                        }
                                } );