Bug 1314861: Some trivial optimizations that add up something significant. r?ochameau draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 02 Nov 2016 12:25:21 -0700
changeset 433088 ce452418dfd8b02daa9286df02fb72827d3ce32d
parent 433087 fb6b8a936888a56fb07dc60deb6955c7cfb42076
child 433089 3164f4293bfbd4ee865c21baeefe00686d6f1057
push id34481
push usermaglione.k@gmail.com
push dateThu, 03 Nov 2016 03:45:04 +0000
reviewersochameau
bugs1314861
milestone52.0a1
Bug 1314861: Some trivial optimizations that add up something significant. r?ochameau MozReview-Commit-ID: FmiH8daYxK8
addon-sdk/source/lib/toolkit/loader.js
--- a/addon-sdk/source/lib/toolkit/loader.js
+++ b/addon-sdk/source/lib/toolkit/loader.js
@@ -48,18 +48,20 @@ defineLazyGetter(this, "XulApp", () => {
                                      "sdk/system/xul-app.jsm");
   return Cu.import(xulappURI, {});
 });
 
 // Define some shortcuts.
 const bind = Function.call.bind(Function.bind);
 const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
 const prototypeOf = Object.getPrototypeOf;
-const getOwnIdentifiers = x => [...Object.getOwnPropertyNames(x),
-                                ...Object.getOwnPropertySymbols(x)];
+function* getOwnIdentifiers(x) {
+  yield* Object.getOwnPropertyNames(x);
+  yield* Object.getOwnPropertySymbols(x);
+}
 
 const NODE_MODULES = new Set([
   "assert",
   "buffer_ieee754",
   "buffer",
   "child_process",
   "cluster",
   "console",
@@ -120,19 +122,18 @@ function freeze(object) {
       freeze(object);
   }
   return object;
 }
 
 // Returns map of given `object`-s own property descriptors.
 const descriptor = iced(function descriptor(object) {
   let value = {};
-  getOwnIdentifiers(object).forEach(function(name) {
+  for (let name of getOwnIdentifiers(object))
     value[name] = getOwnPropertyDescriptor(object, name)
-  });
   return value;
 });
 Loader.descriptor = descriptor;
 
 // Freeze important built-ins so they can't be used by untrusted code as a
 // message passing channel.
 freeze(Object);
 freeze(Object.prototype);
@@ -155,21 +156,21 @@ function iced(f) {
 }
 
 // Defines own properties of given `properties` object on the given
 // target object overriding any existing property with a conflicting name.
 // Returns `target` object. Note we only export this function because it's
 // useful during loader bootstrap when other util modules can't be used &
 // thats only case where this export should be used.
 const override = iced(function override(target, source) {
-  let properties = descriptor(target)
-  let extension = descriptor(source || {})
-  getOwnIdentifiers(extension).forEach(function(name) {
-    properties[name] = extension[name];
-  });
+  let properties = descriptor(target);
+
+  for (let name of getOwnIdentifiers(source || {}))
+    properties[name] = getOwnPropertyDescriptor(source, name);
+
   return Object.defineProperties({}, properties);
 });
 Loader.override = override;
 
 function sourceURI(uri) { return String(uri).split(" -> ").pop(); }
 Loader.sourceURI = iced(sourceURI);
 
 function isntLoaderFrame(frame) { return frame.fileName !== module.uri }
@@ -543,17 +544,17 @@ const load = iced(function load(loader, 
   if (loader.checkCompatibility) {
     let err = XulApp.incompatibility(module);
     if (err) {
       throw err;
     }
   }
 
   if (module.exports === originalExports)
-    freeze(module.exports);
+    Object.freeze(module.exports);
 
   return module;
 });
 Loader.load = load;
 
 // Utility function to normalize module `uri`s so they have `.js` extension.
 function normalizeExt(uri) {
   return isJSURI(uri) ? uri :
@@ -570,25 +571,23 @@ function stripBase(rootURI, string) {
 
 // Utility function to join paths. In common case `base` is a
 // `requirer.uri` but in some cases it may be `baseURI`. In order to
 // avoid complexity we require `baseURI` with a trailing `/`.
 const resolve = iced(function resolve(id, base) {
   if (!isRelative(id))
     return id;
 
-  let baseDir = dirname(base);
-  if (!baseDir)
-    return normalize(id);
+  var baseDir = dirname(base);
 
   let resolved = join(baseDir, id);
 
   // Joining and normalizing removes the './' from relative files.
   // We need to ensure the resolution still has the root
-  if (isRelative(base))
+  if (base.startsWith('./'))
     resolved = './' + resolved;
 
   return resolved;
 });
 Loader.resolve = resolve;
 
 // Attempts to load `path` and then `path.js`
 // Returns `path` with valid file, or `undefined` otherwise
@@ -624,17 +623,17 @@ function resolveAsDirectory(path) {
 }
 
 function resolveRelative(rootURI, modulesDir, id) {
   let fullId = join(rootURI, modulesDir, id);
 
   let resolvedPath = (resolveAsFile(fullId) ||
                       resolveAsDirectory(fullId));
   if (resolvedPath) {
-    return stripBase(rootURI, resolvedPath);
+    return './' + resolvedPath.slice(rootURI.length);
   }
 
   return null;
 }
 
 // From `resolve` module
 // https://github.com/substack/node-resolve/blob/master/lib/node-modules-paths.js
 function* getNodeModulePaths(rootURI, start) {
@@ -840,17 +839,17 @@ const Require = iced(function Require(lo
     // We also freeze module to prevent it from further changes
     // at runtime.
     if (!(uri in modules)) {
       // Many of the loader's functionalities are dependent
       // on modules[uri] being set before loading, so we set it and
       // remove it if we have any errors.
       module = modules[uri] = Module(requirement, uri);
       try {
-        freeze(load(loader, module));
+        Object.freeze(load(loader, module));
       }
       catch (e) {
         // Clear out modules cache so we can throw on a second invalid require
         delete modules[uri];
         // Also clear out the Sandbox that was created
         delete loader.sandboxes[uri];
         throw e;
       }
@@ -867,22 +866,16 @@ const Require = iced(function Require(lo
       throw Error('you must provide a module name when calling require() from '
                   + requirer.id, requirer.uri);
 
     let requirement, uri;
 
     // TODO should get native Firefox modules before doing node-style lookups
     // to save on loading time
     if (isNative) {
-      // If a requireMap is available from `generateMap`, use that to
-      // immediately resolve the node-style mapping.
-      // TODO: write more tests for this use case
-      if (requireMap && requireMap[requirer.id])
-        requirement = requireMap[requirer.id][id];
-
       let { overrides } = manifest.jetpack;
       for (let key in overrides) {
         // ignore any overrides using relative keys
         if (/^[.\/]/.test(key)) {
           continue;
         }
 
         // If the override is for x -> y,
@@ -895,18 +888,16 @@ const Require = iced(function Require(lo
       }
 
       // For native modules, we want to check if it's a module specified
       // in 'modules', like `chrome`, or `@loader` -- if it exists,
       // just set the uri to skip resolution
       if (!requirement && modules[id])
         uri = requirement = id;
 
-      // If no requireMap was provided, or resolution not found in
-      // the requireMap, and not a npm dependency, attempt a runtime lookup
       if (!requirement && !NODE_MODULES.has(id)) {
         // If `isNative` defined, this is using the new, native-style
         // loader, not cuddlefish, so lets resolve using node's algorithm
         // and get back a path that needs to be resolved via paths mapping
         // in `resolveURI`
         requirement = loaderResolve(id, requirer.id, {
           manifest: manifest,
           rootURI: rootURI
@@ -1161,32 +1152,29 @@ function Loader(options) {
         set: function(module) { main = main || module; }
       }
     }
   };
 
   if (isNative) {
     returnObj.isNative = { enumerable: false, value: true };
     returnObj.manifest = { enumerable: false, value: manifest };
-    returnObj.requireMap = { enumerable: false, value: requireMap };
     returnObj.rootURI = { enumerable: false, value: addTrailingSlash(rootURI) };
   }
 
   return freeze(Object.create(null, returnObj));
 };
 Loader.Loader = Loader;
 
 var isSystemURI = uri => /^resource:\/\/(gre|devtools|testing-common)\//.test(uri);
 
 var isJSONURI = uri => uri.endsWith('.json');
 var isJSMURI = uri => uri.endsWith('.jsm');
 var isJSURI = uri => uri.endsWith('.js');
-var isAbsoluteURI = uri => uri.startsWith("resource://") ||
-                           uri.startsWith("chrome://") ||
-                           uri.startsWith("file://");
+var isAbsoluteURI = uri => /^(resource|chrome|file|jar):/.test(uri);
 var isRelative = id => id.startsWith(".");
 
 // Default `main` entry to './index.js' and ensure is relative,
 // since node allows 'lib/index.js' without relative `./`
 function getManifestMain(manifest) {
   let main = manifest.main || './index.js';
   return isRelative(main) ? main : './' + main;
 }