Change nsXPConnect::CheckForDebugMode to trigger one multi-compartment GC instead of individual GCs for all compartments, to fix GC pauses when switching tabs with Firebug. Bug 754201, r=jorendorff, r=sfink.
authorTill Schneidereit <tschneidereit@gmail.com>
Mon, 14 May 2012 18:46:51 -0500
changeset 93985 c00a9c1940c5a9f48a06e4ab7f0222a5a49de7e1
parent 93984 5c90425ab44ea3d066472a0c944531441945f0d7
child 94078 bfc259be3fa25ec40c92665f48dddb9141bb889f
push id22694
push userjorendorff@mozilla.com
push dateWed, 16 May 2012 01:53:44 +0000
treeherdermozilla-central@c00a9c1940c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, sfink
bugs754201
milestone15.0a1
first release with
nightly linux32
c00a9c1940c5 / 15.0a1 / 20120516030515 / files
nightly linux64
c00a9c1940c5 / 15.0a1 / 20120516030515 / files
nightly mac
c00a9c1940c5 / 15.0a1 / 20120516030515 / files
nightly win32
c00a9c1940c5 / 15.0a1 / 20120516030515 / files
nightly win64
c00a9c1940c5 / 15.0a1 / 20120516030515 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Change nsXPConnect::CheckForDebugMode to trigger one multi-compartment GC instead of individual GCs for all compartments, to fix GC pauses when switching tabs with Firebug. Bug 754201, r=jorendorff, r=sfink.
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsdbgapi.cpp
js/src/jsdbgapi.h
js/xpconnect/src/nsXPConnect.cpp
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -594,17 +594,17 @@ JSCompartment::hasScriptsOnStack()
         JSScript *script = i.fp()->maybeScript();
         if (script && script->compartment() == this)
             return true;
     }
     return false;
 }
 
 bool
-JSCompartment::setDebugModeFromC(JSContext *cx, bool b)
+JSCompartment::setDebugModeFromC(JSContext *cx, bool b, AutoDebugModeGC &dmgc)
 {
     bool enabledBefore = debugMode();
     bool enabledAfter = (debugModeBits & ~unsigned(DebugFromC)) || b;
 
     // Debug mode can be enabled only when no scripts from the target
     // compartment are on the stack. It would even be incorrect to discard just
     // the non-live scripts' JITScripts because they might share ICs with live
     // scripts (bug 632343).
@@ -621,22 +621,22 @@ JSCompartment::setDebugModeFromC(JSConte
             JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_NOT_IDLE);
             return false;
         }
     }
 
     debugModeBits = (debugModeBits & ~unsigned(DebugFromC)) | (b ? DebugFromC : 0);
     JS_ASSERT(debugMode() == enabledAfter);
     if (enabledBefore != enabledAfter)
-        updateForDebugMode(cx->runtime->defaultFreeOp());
+        updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc);
     return true;
 }
 
 void
-JSCompartment::updateForDebugMode(FreeOp *fop)
+JSCompartment::updateForDebugMode(FreeOp *fop, AutoDebugModeGC &dmgc)
 {
     for (ContextIter acx(rt); !acx.done(); acx.next()) {
         if (acx->compartment == this) 
             acx->updateJITEnabled();
     }
 
 #ifdef JS_METHODJIT
     bool enabled = debugMode();
@@ -651,34 +651,39 @@ JSCompartment::updateForDebugMode(FreeOp
         mjit::ReleaseScriptCode(fop, script);
         script->debugMode = enabled;
     }
 
     // Discard JIT code and bytecode analysis for all scripts in this
     // compartment. Because !hasScriptsOnStack(), it suffices to do a garbage
     // collection cycle or to finish the ongoing GC cycle. The necessary
     // cleanup happens in JSCompartment::sweep.
-    if (!rt->gcRunning) {
-        PrepareCompartmentForGC(this);
-        GC(rt, GC_NORMAL, gcreason::DEBUG_MODE_GC);
-    }
+    //
+    // dmgc makes sure we can't forget to GC, but it is also important not
+    // to run any scripts in this compartment until the dmgc is destroyed.
+    // That is the caller's responsibility.
+    //
+    if (!rt->gcRunning)
+        dmgc.scheduleGC(this);
 #endif
 }
 
 bool
 JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global)
 {
     bool wasEnabled = debugMode();
     if (!debuggees.put(global)) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     debugModeBits |= DebugFromJS;
-    if (!wasEnabled)
-        updateForDebugMode(cx->runtime->defaultFreeOp());
+    if (!wasEnabled) {
+        AutoDebugModeGC dmgc(cx->runtime);
+        updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc);
+    }
     return true;
 }
 
 void
 JSCompartment::removeDebuggee(FreeOp *fop,
                               js::GlobalObject *global,
                               js::GlobalObjectSet::Enum *debuggeesEnum)
 {
@@ -686,18 +691,20 @@ JSCompartment::removeDebuggee(FreeOp *fo
     JS_ASSERT(debuggees.has(global));
     if (debuggeesEnum)
         debuggeesEnum->removeFront();
     else
         debuggees.remove(global);
 
     if (debuggees.empty()) {
         debugModeBits &= ~DebugFromJS;
-        if (wasEnabled && !debugMode())
-            updateForDebugMode(fop);
+        if (wasEnabled && !debugMode()) {
+            AutoDebugModeGC dmgc(rt);
+            updateForDebugMode(fop, dmgc);
+        }
     }
 }
 
 void
 JSCompartment::clearBreakpointsIn(FreeOp *fop, js::Debugger *dbg, JSObject *handler)
 {
     for (gc::CellIter i(this, gc::FINALIZE_SCRIPT); !i.done(); i.next()) {
         JSScript *script = i.get<JSScript>();
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -107,16 +107,20 @@ struct WrapperHasher
 typedef HashMap<Value, ReadBarrieredValue, WrapperHasher, SystemAllocPolicy> WrapperMap;
 
 } /* namespace js */
 
 namespace JS {
 struct TypeInferenceSizes;
 }
 
+namespace js {
+class AutoDebugModeGC;
+}
+
 struct JSCompartment
 {
     JSRuntime                    *rt;
     JSPrincipals                 *principals;
 
     js::gc::ArenaLists           arenas;
 
   private:
@@ -351,24 +355,24 @@ struct JSCompartment
      */
     bool debugMode() const { return !!debugModeBits; }
 
     /* True if any scripts from this compartment are on the JS stack. */
     bool hasScriptsOnStack();
 
   private:
     /* This is called only when debugMode() has just toggled. */
-    void updateForDebugMode(js::FreeOp *fop);
+    void updateForDebugMode(js::FreeOp *fop, js::AutoDebugModeGC &dmgc);
 
   public:
     js::GlobalObjectSet &getDebuggees() { return debuggees; }
     bool addDebuggee(JSContext *cx, js::GlobalObject *global);
     void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global,
                         js::GlobalObjectSet::Enum *debuggeesEnum = NULL);
-    bool setDebugModeFromC(JSContext *cx, bool b);
+    bool setDebugModeFromC(JSContext *cx, bool b, js::AutoDebugModeGC &dmgc);
 
     void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JSObject *handler);
     void clearTraps(js::FreeOp *fop);
 
   private:
     void sweepBreakpoints(js::FreeOp *fop);
 
   public:
@@ -376,16 +380,39 @@ struct JSCompartment
 
     js::ScriptCountsMap *scriptCountsMap;
 
     js::SourceMapMap *sourceMapMap;
 
     js::DebugScriptMap *debugScriptMap;
 };
 
+// For use when changing the debug mode flag on one or more compartments.
+// Do not run scripts in any compartment that is scheduled for GC using this
+// object. See comment in updateForDebugMode.
+//
+class js::AutoDebugModeGC
+{
+    JSRuntime *rt;
+    bool needGC;
+  public:
+    explicit AutoDebugModeGC(JSRuntime *rt) : rt(rt), needGC(false) {}
+
+    ~AutoDebugModeGC() {
+        if (needGC)
+            GC(rt, GC_NORMAL, gcreason::DEBUG_MODE_GC);
+    }
+
+    void scheduleGC(JSCompartment *compartment) {
+        JS_ASSERT(!rt->gcRunning);
+        PrepareCompartmentForGC(compartment);
+        needGC = true;
+    }
+};
+
 #define JS_PROPERTY_TREE(cx)    ((cx)->compartment->propertyTree)
 
 inline void
 JSContext::setCompartment(JSCompartment *compartment)
 {
     this->compartment = compartment;
     this->inferenceEnabled = compartment ? compartment->types.inferenceEnabled : false;
 }
--- a/js/src/jsdbgapi.cpp
+++ b/js/src/jsdbgapi.cpp
@@ -159,19 +159,35 @@ ScriptDebugEpilogue(JSContext *cx, Stack
     }
 
     return Debugger::onLeaveFrame(cx, ok);
 }
 
 } /* namespace js */
 
 JS_FRIEND_API(JSBool)
+JS_SetDebugModeForAllCompartments(JSContext *cx, JSBool debug)
+{
+    AutoDebugModeGC dmgc(cx->runtime);
+
+    for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) {
+        // Ignore special compartments (atoms, JSD compartments)
+        if (c->principals) {
+            if (!c->setDebugModeFromC(cx, !!debug, dmgc))
+                return false;
+        }
+    }
+    return true;
+}
+
+JS_FRIEND_API(JSBool)
 JS_SetDebugModeForCompartment(JSContext *cx, JSCompartment *comp, JSBool debug)
 {
-    return comp->setDebugModeFromC(cx, !!debug);
+    AutoDebugModeGC dmgc(cx->runtime);
+    return comp->setDebugModeFromC(cx, !!debug, dmgc);
 }
 
 static JSBool
 CheckDebugMode(JSContext *cx)
 {
     JSBool debugMode = JS_GetDebugMode(cx);
     /*
      * :TODO:
--- a/js/src/jsdbgapi.h
+++ b/js/src/jsdbgapi.h
@@ -116,16 +116,23 @@ JS_SetRuntimeDebugMode(JSRuntime *rt, JS
  * enable debug mode while frames are live.
  */
 
 /* Get current state of debugging mode. */
 extern JS_PUBLIC_API(JSBool)
 JS_GetDebugMode(JSContext *cx);
 
 /*
+ * Turn on/off debugging mode for all compartments. This returns false if any code
+ * from any of the runtime's compartments is running or on the stack.
+ */
+JS_FRIEND_API(JSBool)
+JS_SetDebugModeForAllCompartments(JSContext *cx, JSBool debug);
+
+/*
  * Turn on/off debugging mode for a single compartment. This should only be
  * used when no code from this compartment is running or on the stack in any
  * thread.
  */
 JS_FRIEND_API(JSBool)
 JS_SetDebugModeForCompartment(JSContext *cx, JSCompartment *comp, JSBool debug);
 
 /*
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -2400,27 +2400,18 @@ nsXPConnect::CheckForDebugMode(JSRuntime
     {
         struct AutoDestroyContext {
             JSContext *cx;
             AutoDestroyContext(JSContext *cx) : cx(cx) {}
             ~AutoDestroyContext() { JS_DestroyContext(cx); }
         } adc(cx);
         JSAutoRequest ar(cx);
 
-        const js::CompartmentVector &vector = js::GetRuntimeCompartments(rt);
-        for (JSCompartment * const *p = vector.begin(); p != vector.end(); ++p) {
-            JSCompartment *comp = *p;
-            if (!JS_GetCompartmentPrincipals(comp)) {
-                /* Ignore special compartments (atoms, JSD compartments) */
-                continue;
-            }
-
-            if (!JS_SetDebugModeForCompartment(cx, comp, gDesiredDebugMode))
-                goto fail;
-        }
+        if (!JS_SetDebugModeForAllCompartments(cx, gDesiredDebugMode))
+            goto fail;
     }
 
     if (gDesiredDebugMode) {
         rv = jsds->ActivateDebugger(rt);
     }
 
     gDebugMode = gDesiredDebugMode;
     return;