Bug 1464548: Part 1b - Don't delete properties before redefining them, because deleting properties kills JIT performance. r=mccr8
authorKris Maglione <maglione.k@gmail.com>
Fri, 25 May 2018 19:17:58 -0700
changeset 421854 4a8cd93316d6785a593b204bc204d6004449d76a
parent 421853 f738942056e19cbe90efdd8a455e149008ef2deb
child 421855 a6e7bfa38103efc76635dfd8d4ecd6bf51a6c590
push id104131
push usermaglione.k@gmail.com
push dateFri, 08 Jun 2018 00:33:41 +0000
treeherdermozilla-inbound@22daf991f7b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1464548
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1464548: Part 1b - Don't delete properties before redefining them, because deleting properties kills JIT performance. r=mccr8 MozReview-Commit-ID: IUMg59xRoIu
browser/components/places/tests/unit/head_bookmarks.js
browser/components/places/tests/unit/test_PUIU_batchUpdatesForNode.js
js/xpconnect/loader/XPCOMUtils.jsm
--- a/browser/components/places/tests/unit/head_bookmarks.js
+++ b/browser/components/places/tests/unit/head_bookmarks.js
@@ -10,20 +10,18 @@ ChromeUtils.import("resource://gre/modul
 var commonFile = do_get_file("../../../../../toolkit/components/places/tests/head_common.js", false);
 if (commonFile) {
   let uri = Services.io.newFileURI(commonFile);
   Services.scriptloader.loadSubScript(uri.spec, this);
 }
 
 // Put any other stuff relative to this test folder below.
 
-XPCOMUtils.defineLazyGetter(this, "PlacesUIUtils", function() {
-  ChromeUtils.import("resource:///modules/PlacesUIUtils.jsm");
-  return PlacesUIUtils;
-});
+ChromeUtils.defineModuleGetter(this, "PlacesUIUtils",
+                               "resource:///modules/PlacesUIUtils.jsm");
 
 // Needed by some test that relies on having an app registered.
 ChromeUtils.import("resource://testing-common/AppInfo.jsm", this);
 updateAppInfo({
   name: "PlacesTest",
   ID: "{230de50e-4cd1-11dc-8314-0800200c9a66}",
   version: "1",
   platformVersion: "",
--- a/browser/components/places/tests/unit/test_PUIU_batchUpdatesForNode.js
+++ b/browser/components/places/tests/unit/test_PUIU_batchUpdatesForNode.js
@@ -1,18 +1,16 @@
 // ================================================
 // Load mocking/stubbing library, sinon
 // docs: http://sinonjs.org/releases/v2.3.2/
 ChromeUtils.import("resource://gre/modules/Timer.jsm");
 Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js", this);
 /* globals sinon */
 // ================================================
 
-ChromeUtils.import("resource:///modules/PlacesUIUtils.jsm");
-
 /* eslint-disable mozilla/use-chromeutils-generateqi */
 
 add_task(async function test_no_result_node() {
   let functionSpy = sinon.stub().returns(Promise.resolve());
 
   await PlacesUIUtils.batchUpdatesForNode(null, 1, functionSpy);
 
   Assert.ok(functionSpy.calledOnce,
--- a/js/xpconnect/loader/XPCOMUtils.jsm
+++ b/js/xpconnect/loader/XPCOMUtils.jsm
@@ -87,16 +87,31 @@
  *  this.NSGetFactory = XPCOMUtils.generateNSGetFactory(components);
  */
 
 
 var EXPORTED_SYMBOLS = [ "XPCOMUtils" ];
 
 let global = Cu.getGlobalForObject({});
 
+/**
+ * Redefines the given property on the given object with the given
+ * value. This can be used to redefine getter properties which do not
+ * implement setters.
+ */
+function redefine(object, prop, value) {
+  Object.defineProperty(object, prop, {
+    configurable: true,
+    enumerable: true,
+    value,
+    writable: true,
+  });
+  return value;
+}
+
 var XPCOMUtils = {
   /**
    * Generate a QueryInterface implementation. The returned function must be
    * assigned to the 'QueryInterface' property of a JS object. When invoked on
    * that object, it checks if the given iid is listed in the |interfaces|
    * param, and if it is, returns |this| (the object it was called on).
    * If the JS object has a classInfo property it'll be returned for the
    * nsIClassInfo IID, generateCI can be used to generate the classInfo
@@ -170,31 +185,25 @@ var XPCOMUtils = {
    * @param aName
    *        The name of the getter to define on aObject.
    * @param aLambda
    *        A function that returns what the getter should return.  This will
    *        only ever be called once.
    */
   defineLazyGetter: function XPCU_defineLazyGetter(aObject, aName, aLambda)
   {
+    let redefining = false;
     Object.defineProperty(aObject, aName, {
       get: function () {
-        // Redefine this accessor property as a data property.
-        // Delete it first, to rule out "too much recursion" in case aObject is
-        // a proxy whose defineProperty handler might unwittingly trigger this
-        // getter again.
-        delete aObject[aName];
-        let value = aLambda.apply(aObject);
-        Object.defineProperty(aObject, aName, {
-          value,
-          writable: true,
-          configurable: true,
-          enumerable: true
-        });
-        return value;
+        if (!redefining) {
+          // Make sure we don't get into an infinite recursion loop if
+          // the getter lambda does something shady.
+          redefining = true;
+          return redefine(aObject, aName, aLambda.apply(aObject));
+        }
       },
       configurable: true,
       enumerable: true
     });
   },
 
   /**
    * Defines a getter on a specified object for a script.  The script will not
@@ -214,22 +223,22 @@ var XPCOMUtils = {
                                                                aResource)
   {
     if (!Array.isArray(aNames)) {
       aNames = [aNames];
     }
     for (let name of aNames) {
       Object.defineProperty(aObject, name, {
         get: function() {
-          for (let n of aNames) {
-            delete aObject[n];
-          }
           Services.scriptloader.loadSubScript(aResource, aObject);
           return aObject[name];
         },
+        set(value) {
+          redefine(aObject, name, value);
+        },
         configurable: true,
         enumerable: true
       });
     }
   },
 
   /**
    * Defines a getter property on the given object for each of the given