Bug 1490009 - Clear CallbackObject fields after use for promise job to avoid tenuring objects unnecessarily r=bz a=pascalc
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 20 Sep 2018 13:28:59 +0100
changeset 490161 01dd14f85e40ed9ffadb2296aa305c4c7022e99c
parent 490160 b3fad6a4c6cb3b26e1692ef9caa42e965564dc0e
child 490162 c2b5cb2f178c01676790a35a2392376e7a0191cb
push id9929
push userapavel@mozilla.com
push dateWed, 03 Oct 2018 13:05:32 +0000
treeherdermozilla-beta@01dd14f85e40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, pascalc
bugs1490009
milestone63.0
Bug 1490009 - Clear CallbackObject fields after use for promise job to avoid tenuring objects unnecessarily r=bz a=pascalc
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
@@ -223,16 +223,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();
   }