Bug 1379957 - Only fire the debugger's onGarbageCollection hook when necessary to avoid extra worker GCs r=fitzgen
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 12 Jul 2017 18:31:56 +0100
changeset 607724 872e9c8fec61fbc0596db21b3002472dea32d864
parent 607723 933aa2989b9af7f7746c5b45b41c5ff3ce52cd20
child 607725 9b778f1f3c2d8ac62c12c7b3f510962e940256bf
push id68095
push userbmo:rbarker@mozilla.com
push dateWed, 12 Jul 2017 20:01:47 +0000
reviewersfitzgen
bugs1379957
milestone56.0a1
Bug 1379957 - Only fire the debugger's onGarbageCollection hook when necessary to avoid extra worker GCs r=fitzgen
js/public/Debug.h
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/js/public/Debug.h
+++ b/js/public/Debug.h
@@ -274,16 +274,21 @@ GetDebuggerMallocSizeOf(JSContext* cx);
 //
 // The Debugger wants to report about its debuggees' GC cycles, however entering
 // JS after a GC is troublesome since SpiderMonkey will often do something like
 // force a GC and then rely on the nursery being empty. If we call into some
 // Debugger's hook after the GC, then JS runs and the nursery won't be
 // empty. Instead, we rely on embedders to call back into SpiderMonkey after a
 // GC and notify Debuggers to call their onGarbageCollection hook.
 
+// Determine whether it's necessary to call FireOnGarbageCollectionHook() after
+// a GC. This is only required if there are debuggers with an
+// onGarbageCollection hook observing a global in the set of collected zones.
+JS_PUBLIC_API(bool)
+FireOnGarbageCollectionHookRequired(JSContext* cx);
 
 // For each Debugger that observed a debuggee involved in the given GC event,
 // call its `onGarbageCollection` hook.
 JS_PUBLIC_API(bool)
 FireOnGarbageCollectionHook(JSContext* cx, GarbageCollectionEvent::Ptr&& data);
 
 
 
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -11958,16 +11958,35 @@ GarbageCollectionEvent::toJSObject(JSCon
     RootedValue slicesValue(cx, ObjectValue(*slicesArray));
     if (!DefineProperty(cx, obj, cx->names().collections, slicesValue))
         return nullptr;
 
     return obj;
 }
 
 JS_PUBLIC_API(bool)
+FireOnGarbageCollectionHookRequired(JSContext* cx)
+{
+    AutoCheckCannotGC noGC;
+
+    for (ZoneGroupsIter group(cx->runtime()); !group.done(); group.next()) {
+        for (Debugger* dbg : group->debuggerList()) {
+            if (dbg->enabled &&
+                dbg->observedGC(cx->runtime()->gc.majorGCCount()) &&
+                dbg->getHook(Debugger::OnGarbageCollection))
+            {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
+JS_PUBLIC_API(bool)
 FireOnGarbageCollectionHook(JSContext* cx, JS::dbg::GarbageCollectionEvent::Ptr&& data)
 {
     AutoObjectVector triggered(cx);
 
     {
         // We had better not GC (and potentially get a dangling Debugger
         // pointer) while finding all Debuggers observing a debuggee that
         // participated in this GC.
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -265,16 +265,17 @@ class Debugger : private mozilla::Linked
     friend class ScriptedOnPopHandler;
     friend class mozilla::LinkedListElement<Debugger>;
     friend class mozilla::LinkedList<Debugger>;
     friend bool (::JS_DefineDebuggerObject)(JSContext* cx, JS::HandleObject obj);
     friend bool (::JS::dbg::IsDebugger)(JSObject&);
     friend bool (::JS::dbg::GetDebuggeeGlobals)(JSContext*, JSObject&, AutoObjectVector&);
     friend void JS::dbg::onNewPromise(JSContext* cx, HandleObject promise);
     friend void JS::dbg::onPromiseSettled(JSContext* cx, HandleObject promise);
+    friend bool JS::dbg::FireOnGarbageCollectionHookRequired(JSContext* cx);
     friend bool JS::dbg::FireOnGarbageCollectionHook(JSContext* cx,
                                                      JS::dbg::GarbageCollectionEvent::Ptr&& data);
 
   public:
     enum Hook {
         OnDebuggerStatement,
         OnExceptionUnwind,
         OnNewScript,
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -844,17 +844,18 @@ CycleCollectedJSRuntime::GCSliceCallback
       profiler_add_marker(
         "GCSlice",
         MakeUnique<GCSliceMarkerPayload>(aDesc.lastSliceStart(aContext),
                                          aDesc.lastSliceEnd(aContext),
                                          aDesc.sliceToJSON(aContext)));
     }
   }
 
-  if (aProgress == JS::GC_CYCLE_END) {
+  if (aProgress == JS::GC_CYCLE_END &&
+      JS::dbg::FireOnGarbageCollectionHookRequired(aContext)) {
     JS::gcreason::Reason reason = aDesc.reason_;
     Unused <<
       NS_WARN_IF(NS_FAILED(DebuggerOnGCRunnable::Enqueue(aContext, aDesc)) &&
                  reason != JS::gcreason::SHUTDOWN_CC &&
                  reason != JS::gcreason::DESTROY_RUNTIME &&
                  reason != JS::gcreason::XPCONNECT_SHUTDOWN);
   }