Bug 1490009 - Clear CallbackObject fields after use for promise job to avoid tenuring objects unnecessarily r=bz
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 20 Sep 2018 13:28:59 +0100
changeset 494897 cae1a7f33840ded4f2632eb18ed3a564bce2accf
parent 494896 d62fe627f30fdb82bd06c7f10d3ca6599d298d18
child 494898 cdd5b0bde9c1cc29213e1fb82ce3688799cb8af8
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1490009
milestone64.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 1490009 - Clear CallbackObject fields after use for promise job to avoid tenuring objects unnecessarily r=bz
dom/bindings/CallbackObject.h
xpcom/base/CycleCollectedJSContext.cpp
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -29,16 +29,19 @@
 #include "mozilla/dom/ScriptSettings.h"
 #include "nsWrapperCache.h"
 #include "nsJSEnvironment.h"
 #include "xpcpublic.h"
 #include "jsapi.h"
 #include "js/TracingAPI.h"
 
 namespace mozilla {
+
+class PromiseJobRunnable;
+
 namespace dom {
 
 #define DOM_CALLBACKOBJECT_IID \
 { 0xbe74c190, 0x6d76, 0x4991, \
  { 0x84, 0xb9, 0x65, 0x06, 0x99, 0xe6, 0x93, 0x2b } }
 
 class CallbackObject : public nsISupports
 {
@@ -78,18 +81,19 @@ public:
                           JSObject* aAsyncStack,
                           nsIGlobalObject* aIncumbentGlobal)
   {
     Init(aCallback, aCallbackGlobal, aAsyncStack, aIncumbentGlobal);
   }
 
   // This is guaranteed to be non-null from the time the CallbackObject is
   // created until JavaScript has had a chance to run. It will only return null
-  // after a JavaScript caller has called nukeSandbox on a Sandbox object, and
-  // the cycle collector has had a chance to run.
+  // after a JavaScript caller has called nukeSandbox on a Sandbox object and
+  // the cycle collector has had a chance to run, unless Reset() is explicitly
+  // called (see below).
   //
   // This means that any native callee which receives a CallbackObject as an
   // argument can safely rely on the callback being non-null so long as it
   // doesn't trigger any scripts before it accesses it.
   JS::Handle<JSObject*> CallbackOrNull() const
   {
     mCallback.exposeToActiveJS();
     return CallbackPreserveColor();
@@ -232,16 +236,26 @@ private:
                    nsIGlobalObject* aIncumbentGlobal)
   {
     // Set script objects before we hold, on the off chance that a GC could
     // somehow happen in there... (which would be pretty odd, granted).
     InitNoHold(aCallback, aCallbackGlobal, aCreationStack, aIncumbentGlobal);
     mozilla::HoldJSObjects(this);
   }
 
+  // Provide a way to clear this object's pointers to GC things after the
+  // callback has been run. Note that CallbackOrNull() will return null after
+  // this point. This should only be called if the object is known not to be
+  // used again.
+  void Reset()
+  {
+    ClearJSReferences();
+  }
+  friend class mozilla::PromiseJobRunnable;
+
   inline void ClearJSReferences()
   {
     mCallback = nullptr;
     mCallbackGlobal = nullptr;
     mCreationStack = nullptr;
     mIncumbentJSGlobal = nullptr;
   }
 
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -222,16 +222,22 @@ protected:
   virtual void Run(AutoSlowOperation& aAso) override
   {
     JSObject* callback = mCallback->CallbackPreserveColor();
     nsIGlobalObject* global = callback ? xpc::NativeGlobal(callback) : nullptr;
     if (global && !global->IsDying()) {
       mCallback->Call("promise callback");
       aAso.CheckForInterrupt();
     }
+    // Now that mCallback is no longer needed, clear any pointers it contains to
+    // JS GC things. This removes any storebuffer entries associated with those
+    // pointers, which can cause problems by taking up memory and by triggering
+    // minor GCs. This otherwise would not happen until the next minor GC or
+    // cycle collection.
+    mCallback->Reset();
   }
 
   virtual bool Suppressed() override
   {
     nsIGlobalObject* global =
       xpc::NativeGlobal(mCallback->CallbackPreserveColor());
     return global && global->IsInSyncOperation();
   }