Bug 1314861: Add caching to some of the more expensive and repetitive parts of module resolution. r?ochameau draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 07 Apr 2017 18:22:33 -0700
changeset 558814 126d931d4e5787a7e4038b2b15bcca66f3bb7d9f
parent 558813 22e1611c9ba2e557344be3d32c783b192c7251e8
child 558815 069cb0199193eec8a80a35f78e50da548accf1bc
push id52953
push usermaglione.k@gmail.com
push dateSat, 08 Apr 2017 01:32:07 +0000
reviewersochameau
bugs1314861
milestone55.0a1
Bug 1314861: Add caching to some of the more expensive and repetitive parts of module resolution. r?ochameau MozReview-Commit-ID: 8vHcu4c8jDe
addon-sdk/source/lib/toolkit/loader.js
--- a/addon-sdk/source/lib/toolkit/loader.js
+++ b/addon-sdk/source/lib/toolkit/loader.js
@@ -279,23 +279,43 @@ const urlCache = {
       let uri = NetUtil.newURI(url).QueryInterface(Ci.nsIFileURL);
 
       return uri.file.exists();
     } catch (e) {
       return false;
     }
   }),
 
+  resolutionCache: new DefaultMap(fullId => {
+    return (resolveAsFile(fullId) ||
+            resolveAsDirectory(fullId));
+  }),
+
+  nodeModulesCache: new Map(),
+
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupportsWeakReference]),
 
   observe() {
     // Clear any module resolution caches when the startup cache is flushed,
     // since it probably means we're loading new copies of extensions.
     this.zipContentsCache.clear();
     this.filesCache.clear();
+    this.resolutionCache.clear();
+    this.nodeModulesCache.clear();
+  },
+
+  getNodeModulePaths(rootURI, start) {
+    let url = join(rootURI, start);
+
+    if (this.nodeModulesCache.has(url))
+      return this.nodeModulesCache.get(url);
+
+    let result = Array.from(getNodeModulePaths(rootURI, start));
+    this.nodeModulesCache.set(url, result);
+    return result;
   },
 
   /**
    * Returns the base URL for the given URL, if one can be determined. For
    * a resource: URL, this is the root of the resource package. For a jar:
    * URL, it is the root of the JAR file. Otherwise, null is returned.
    *
    * @param {string} url
@@ -621,18 +641,17 @@ function resolveAsDirectory(path) {
   } catch (e) {}
 
   return resolveAsFile(addTrailingSlash(path) + 'index.js');
 }
 
 function resolveRelative(rootURI, modulesDir, id) {
   let fullId = join(rootURI, modulesDir, id);
 
-  let resolvedPath = (resolveAsFile(fullId) ||
-                      resolveAsDirectory(fullId));
+  let resolvedPath = urlCache.resolutionCache.get(fullId);
   if (resolvedPath) {
     return './' + resolvedPath.slice(rootURI.length);
   }
 
   return null;
 }
 
 // From `resolve` module
@@ -679,17 +698,17 @@ const nodeResolve = iced(function nodeRe
   // If the requirer is an absolute URI then the node module resolution below
   // won't work correctly as we prefix everything with rootURI
   if (isAbsoluteURI(requirer)) {
     return null;
   }
 
   // If manifest has dependencies, attempt to look up node modules
   // in the `dependencies` list
-  for (let modulesDir of getNodeModulePaths(rootURI, dirname(requirer))) {
+  for (let modulesDir of urlCache.getNodeModulePaths(rootURI, dirname(requirer))) {
     if ((resolvedPath = resolveRelative(rootURI, modulesDir, id))) {
       return resolvedPath;
     }
   }
 
   // We would not find lookup for things like `sdk/tabs`, as that's part of
   // the alias mapping. If during `generateMap`, the runtime lookup resolves
   // with `resolveURI` -- if during runtime, then `resolve` will throw.
@@ -797,19 +816,18 @@ function lazyRequireModule(obj, moduleId
 
 
 // Creates version of `require` that will be exposed to the given `module`
 // in the context of the given `loader`. Each module gets own limited copy
 // of `require` that is allowed to load only a modules that are associated
 // with it during link time.
 const Require = iced(function Require(loader, requirer) {
   let {
-    modules, mapping, resolve: loaderResolve, load,
-    manifest, rootURI, isNative, requireMap,
-    requireHook
+    modules, mapping, mappingCache, resolve: loaderResolve, load,
+    manifest, rootURI, isNative, requireHook
   } = loader;
 
   if (isSystemURI(requirer.uri)) {
     // Built-in modules don't require the expensive module resolution
     // algorithm used by SDK add-ons, so give them the more efficient standard
     // resolve instead.
     isNative = false;
     loaderResolve = Loader.resolve;
@@ -824,16 +842,17 @@ const Require = iced(function Require(lo
       return requireHook(id, _require);
     }
 
     return _require(id);
   }
 
   function _require(id) {
     let { uri, requirement } = getRequirements(id);
+
     let module = null;
     // If module is already cached by loader then just use it.
     if (uri in modules) {
       module = modules[uri];
     }
     else if (isJSMURI(uri)) {
       module = modules[uri] = Module(requirement, uri);
       module.exports = Cu.import(uri, {});
@@ -945,17 +964,24 @@ const Require = iced(function Require(lo
       // Resolve `id` to its requirer if it's relative.
       requirement = loaderResolve(id, requirer.id);
     }
     else {
       requirement = id;
     }
 
     // Resolves `uri` of module using loaders resolve function.
-    uri = uri || resolveURI(requirement, mapping);
+    if (!uri) {
+      if (mappingCache.has(requirement)) {
+        uri = mappingCache.get(requirement);
+      } else {
+        uri = resolveURI(requirement, mapping);
+        mappingCache.set(requirement, uri);
+      }
+    }
 
     // Throw if `uri` can not be resolved.
     if (!uri) {
       throw Error('Module: Can not resolve "' + id + '" module required by ' +
                   requirer.id + ' located at ' + requirer.uri, requirer.uri);
     }
 
     return { uri: uri, requirement: requirement };
@@ -1028,21 +1054,25 @@ Loader.unload = unload;
 //   These modules will incorporated into module cache. Each module will be
 //   frozen.
 // - `resolve` Optional module `id` resolution function. If given it will be
 //   used to resolve module URIs, by calling it with require term, requirer
 //   module object (that has `uri` property) and `baseURI` of the loader.
 //   If `resolve` does not returns `uri` string exception will be thrown by
 //   an associated `require` call.
 function Loader(options) {
+  function normalizeRootURI(uri) {
+    return addTrailingSlash(join(uri));
+  }
+
   if (options.sharedGlobalBlacklist && !options.sharedGlobalBlocklist) {
     options.sharedGlobalBlocklist = options.sharedGlobalBlacklist;
   }
   let {
-    modules, globals, resolve, paths, rootURI, manifest, requireMap, isNative,
+    modules, globals, resolve, paths, rootURI, manifest, isNative,
     metadata, sharedGlobal, sharedGlobalBlocklist, checkCompatibility, waiveIntereposition
   } = override({
     paths: {},
     modules: {},
     globals: {
       get console() {
         // Import Console.jsm from here to prevent loading it until someone uses it
         let { ConsoleAPI } = Cu.import("resource://gre/modules/Console.jsm");
@@ -1051,17 +1081,17 @@ function Loader(options) {
         });
         Object.defineProperty(this, "console", { value: console });
         return this.console;
       }
     },
     checkCompatibility: false,
     resolve: options.isNative ?
       // Make the returned resolve function have the same signature
-      (id, requirer) => Loader.nodeResolve(id, requirer, { rootURI: rootURI }) :
+      (id, requirer) => Loader.nodeResolve(id, requirer, { rootURI: normalizeRootURI(rootURI) }) :
       Loader.resolve,
     sharedGlobalBlocklist: ["sdk/indexed-db"],
     waiveIntereposition: false
   }, options);
 
   // Create overrides defaults, none at the moment
   if (typeof manifest != "object" || !manifest) {
     manifest = {};
@@ -1151,16 +1181,17 @@ function Loader(options) {
 
   // Loader object is just a representation of a environment
   // state. We freeze it and mark make it's properties non-enumerable
   // as they are pure implementation detail that no one should rely upon.
   let returnObj = {
     destructor: { enumerable: false, value: destructor },
     globals: { enumerable: false, value: globals },
     mapping: { enumerable: false, value: mapping },
+    mappingCache: { enumerable: false, value: new Map() },
     // Map of module objects indexed by module URIs.
     modules: { enumerable: false, value: modules },
     metadata: { enumerable: false, value: metadata },
     useSharedGlobalSandbox: { enumerable: false, value: !!sharedGlobal },
     sharedGlobalSandbox: { enumerable: false, value: sharedGlobalSandbox },
     sharedGlobalBlocklist: { enumerable: false, value: sharedGlobalBlocklist },
     sharedGlobalBlacklist: { enumerable: false, value: sharedGlobalBlocklist },
     // Map of module sandboxes indexed by module URIs.
@@ -1186,17 +1217,17 @@ function Loader(options) {
         set: function(module) { main = main || module; }
       }
     }
   };
 
   if (isNative) {
     returnObj.isNative = { enumerable: false, value: true };
     returnObj.manifest = { enumerable: false, value: manifest };
-    returnObj.rootURI = { enumerable: false, value: addTrailingSlash(rootURI) };
+    returnObj.rootURI = { enumerable: false, value: normalizeRootURI(rootURI) };
   }
 
   return freeze(Object.create(null, returnObj));
 };
 Loader.Loader = Loader;
 
 var isSystemURI = uri => /^resource:\/\/(gre|devtools|testing-common)\//.test(uri);