From f07bc4f1320eba9e840747b4267063aa4938b166 Mon Sep 17 00:00:00 2001 From: Panos Astithas Date: Fri, 4 Jan 2013 21:34:43 +0200 Subject: [PATCH] Implement a new addAllGlobalsAsDebuggees method for faster chrome debugging (bug 821701); r=jimb Also GC only once when going through all compartments in both addAllGlobalsAsDebuggees and removeAllDebuggees, instead of once for every debuggee added or removed. --- browser/devtools/debugger/test/Makefile.in | 2 +- .../test/browser_dbg_chrome-debugging.js | 3 -- .../tests/debug/Debugger-debuggees-20.js | 28 +++++++++++ js/src/jscompartment.cpp | 21 ++++++++- js/src/jscompartment.h | 5 ++ js/src/vm/Debugger.cpp | 47 +++++++++++++++++-- js/src/vm/Debugger.h | 7 +++ .../debugger/server/dbg-script-actors.js | 6 +-- 8 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Debugger-debuggees-20.js diff --git a/browser/devtools/debugger/test/Makefile.in b/browser/devtools/debugger/test/Makefile.in index e641bef4d3cf..c71dba794f48 100644 --- a/browser/devtools/debugger/test/Makefile.in +++ b/browser/devtools/debugger/test/Makefile.in @@ -87,7 +87,7 @@ MOCHITEST_BROWSER_TESTS = \ browser_dbg_breakpoint-new-script.js \ browser_dbg_bug737803_editor_actual_location.js \ browser_dbg_progress-listener-bug.js \ - $(filter disabled-for-intermittent-crashes--bug-821701, browser_dbg_chrome-debugging.js) \ + browser_dbg_chrome-debugging.js \ $(filter disabled-for-intermittent-failures--bug-753225, browser_dbg_createRemote.js) \ head.js \ $(NULL) diff --git a/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js b/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js index 1342791aed43..faf3d351224b 100644 --- a/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js +++ b/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js @@ -16,9 +16,6 @@ const DEBUGGER_TAB_URL = EXAMPLE_URL + "browser_dbg_debuggerstatement.html"; function test() { - // Make sure there is enough time for findAllGlobals. - requestLongerTimeout(3); - let transport = DebuggerServer.connectPipe(); gClient = new DebuggerClient(transport); gClient.connect(function(aType, aTraits) { diff --git a/js/src/jit-test/tests/debug/Debugger-debuggees-20.js b/js/src/jit-test/tests/debug/Debugger-debuggees-20.js new file mode 100644 index 000000000000..16319f8a2ca9 --- /dev/null +++ b/js/src/jit-test/tests/debug/Debugger-debuggees-20.js @@ -0,0 +1,28 @@ +// addAllGlobalsAsDebuggees adds all the globals as debuggees. + +var g1 = newGlobal(); // Created before the Debugger; debuggee. +var g2 = newGlobal(); // Created before the Debugger; not debuggee. + +var dbg = new Debugger; + +var g3 = newGlobal(); // Created after the Debugger; debuggee. +var g4 = newGlobal(); // Created after the Debugger; not debuggee. + +var g1w = dbg.addDebuggee(g1); +assertEq(dbg.addAllGlobalsAsDebuggees(), undefined); + +// Get Debugger.Objects viewing the globals from their own compartments; +// this is the sort that findAllGlobals and addDebuggee return. +var g1w = g1w.makeDebuggeeValue(g1).unwrap(); +var g2w = g1w.makeDebuggeeValue(g2).unwrap(); +var g3w = g1w.makeDebuggeeValue(g3).unwrap(); +var g4w = g1w.makeDebuggeeValue(g4).unwrap(); +var thisw = g1w.makeDebuggeeValue(this).unwrap(); + +// Check that they're all there. +assertEq(dbg.hasDebuggee(g1w), true); +assertEq(dbg.hasDebuggee(g2w), true); +assertEq(dbg.hasDebuggee(g3w), true); +assertEq(dbg.hasDebuggee(g4w), true); +// The debugger's global is not a debuggee. +assertEq(dbg.hasDebuggee(thisw), false); diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index 2a2c3baf0dbf..03c34df2b4dd 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -883,6 +883,15 @@ JSCompartment::updateForDebugMode(FreeOp *fop, AutoDebugModeGC &dmgc) bool JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global) +{ + AutoDebugModeGC dmgc(cx->runtime); + return addDebuggee(cx, global, dmgc); +} + +bool +JSCompartment::addDebuggee(JSContext *cx, + js::GlobalObject *global, + AutoDebugModeGC &dmgc) { bool wasEnabled = debugMode(); if (!debuggees.put(global)) { @@ -891,7 +900,6 @@ JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global) } debugModeBits |= DebugFromJS; if (!wasEnabled) { - AutoDebugModeGC dmgc(cx->runtime); updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc); } return true; @@ -901,6 +909,16 @@ void JSCompartment::removeDebuggee(FreeOp *fop, js::GlobalObject *global, js::GlobalObjectSet::Enum *debuggeesEnum) +{ + AutoDebugModeGC dmgc(rt); + return removeDebuggee(fop, global, dmgc, debuggeesEnum); +} + +void +JSCompartment::removeDebuggee(FreeOp *fop, + js::GlobalObject *global, + AutoDebugModeGC &dmgc, + js::GlobalObjectSet::Enum *debuggeesEnum) { bool wasEnabled = debugMode(); JS_ASSERT(debuggees.has(global)); @@ -912,7 +930,6 @@ JSCompartment::removeDebuggee(FreeOp *fop, if (debuggees.empty()) { debugModeBits &= ~DebugFromJS; if (wasEnabled && !debugMode()) { - AutoDebugModeGC dmgc(rt); DebugScopes::onCompartmentLeaveDebugMode(this); updateForDebugMode(fop, dmgc); } diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index ef30980e1664..95e95274097f 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -499,8 +499,13 @@ struct JSCompartment : private JS::shadow::Compartment, public js::gc::GraphNode public: js::GlobalObjectSet &getDebuggees() { return debuggees; } bool addDebuggee(JSContext *cx, js::GlobalObject *global); + bool addDebuggee(JSContext *cx, js::GlobalObject *global, + js::AutoDebugModeGC &dmgc); void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global, js::GlobalObjectSet::Enum *debuggeesEnum = NULL); + void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global, + js::AutoDebugModeGC &dmgc, + js::GlobalObjectSet::Enum *debuggeesEnum = NULL); bool setDebugModeFromC(JSContext *cx, bool b, js::AutoDebugModeGC &dmgc); void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JSObject *handler); diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 1e4e3ea6d346..76a63cbdf03f 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -1881,6 +1881,26 @@ Debugger::addDebuggee(JSContext *cx, unsigned argc, Value *vp) return true; } +JSBool +Debugger::addAllGlobalsAsDebuggees(JSContext *cx, unsigned argc, Value *vp) +{ + THIS_DEBUGGER(cx, argc, vp, "addAllGlobalsAsDebuggees", args, dbg); + AutoDebugModeGC dmgc(cx->runtime); + for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) { + if (c == dbg->object->compartment()) + continue; + c->scheduledForDestruction = false; + GlobalObject *global = c->maybeGlobal(); + if (global) { + Rooted rg(cx, global); + dbg->addDebuggeeGlobal(cx, rg, dmgc); + } + } + + args.rval().setUndefined(); + return true; +} + JSBool Debugger::removeDebuggee(JSContext *cx, unsigned argc, Value *vp) { @@ -1899,8 +1919,9 @@ JSBool Debugger::removeAllDebuggees(JSContext *cx, unsigned argc, Value *vp) { THIS_DEBUGGER(cx, argc, vp, "removeAllDebuggees", args, dbg); + AutoDebugModeGC dmgc(cx->runtime); for (GlobalObjectSet::Enum e(dbg->debuggees); !e.empty(); e.popFront()) - dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), e.front(), NULL, &e); + dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), e.front(), dmgc, NULL, &e); args.rval().setUndefined(); return true; } @@ -2026,6 +2047,15 @@ Debugger::construct(JSContext *cx, unsigned argc, Value *vp) bool Debugger::addDebuggeeGlobal(JSContext *cx, Handle global) +{ + AutoDebugModeGC dmgc(cx->runtime); + return addDebuggeeGlobal(cx, global, dmgc); +} + +bool +Debugger::addDebuggeeGlobal(JSContext *cx, + Handle global, + AutoDebugModeGC &dmgc) { if (debuggees.has(global)) return true; @@ -2082,7 +2112,7 @@ Debugger::addDebuggeeGlobal(JSContext *cx, Handle global) } else { if (global->getDebuggers()->length() > 1) return true; - if (debuggeeCompartment->addDebuggee(cx, global)) + if (debuggeeCompartment->addDebuggee(cx, global, dmgc)) return true; /* Maintain consistency on error. */ @@ -2098,6 +2128,16 @@ void Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global, GlobalObjectSet::Enum *compartmentEnum, GlobalObjectSet::Enum *debugEnum) +{ + AutoDebugModeGC dmgc(global->compartment()->rt); + return removeDebuggeeGlobal(fop, global, dmgc, compartmentEnum, debugEnum); +} + +void +Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global, + AutoDebugModeGC &dmgc, + GlobalObjectSet::Enum *compartmentEnum, + GlobalObjectSet::Enum *debugEnum) { /* * Each debuggee is in two HashSets: one for its compartment and one for @@ -2151,7 +2191,7 @@ Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global, * global cannot be rooted on the stack without a cx. */ if (v->empty()) - global->compartment()->removeDebuggee(fop, global, compartmentEnum); + global->compartment()->removeDebuggee(fop, global, dmgc, compartmentEnum); } /* @@ -2538,6 +2578,7 @@ JSPropertySpec Debugger::properties[] = { JSFunctionSpec Debugger::methods[] = { JS_FN("addDebuggee", Debugger::addDebuggee, 1, 0), + JS_FN("addAllGlobalsAsDebuggees", Debugger::addAllGlobalsAsDebuggees, 0, 0), JS_FN("removeDebuggee", Debugger::removeDebuggee, 1, 0), JS_FN("removeAllDebuggees", Debugger::removeAllDebuggees, 0, 0), JS_FN("hasDebuggee", Debugger::hasDebuggee, 1, 0), diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index 084a60d16863..354cda66f51d 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -225,9 +225,15 @@ class Debugger : private mozilla::LinkedListElement class ScriptQuery; bool addDebuggeeGlobal(JSContext *cx, Handle obj); + bool addDebuggeeGlobal(JSContext *cx, Handle obj, + AutoDebugModeGC &dmgc); void removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global, GlobalObjectSet::Enum *compartmentEnum, GlobalObjectSet::Enum *debugEnum); + void removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global, + AutoDebugModeGC &dmgc, + GlobalObjectSet::Enum *compartmentEnum, + GlobalObjectSet::Enum *debugEnum); /* * Cope with an error or exception in a debugger hook. @@ -300,6 +306,7 @@ class Debugger : private mozilla::LinkedListElement static JSBool getUncaughtExceptionHook(JSContext *cx, unsigned argc, Value *vp); static JSBool setUncaughtExceptionHook(JSContext *cx, unsigned argc, Value *vp); static JSBool addDebuggee(JSContext *cx, unsigned argc, Value *vp); + static JSBool addAllGlobalsAsDebuggees(JSContext *cx, unsigned argc, Value *vp); static JSBool removeDebuggee(JSContext *cx, unsigned argc, Value *vp); static JSBool removeAllDebuggees(JSContext *cx, unsigned argc, Value *vp); static JSBool hasDebuggee(JSContext *cx, unsigned argc, Value *vp); diff --git a/toolkit/devtools/debugger/server/dbg-script-actors.js b/toolkit/devtools/debugger/server/dbg-script-actors.js index 0c7aafa9704a..9c4ae00ce5da 100644 --- a/toolkit/devtools/debugger/server/dbg-script-actors.js +++ b/toolkit/devtools/debugger/server/dbg-script-actors.js @@ -2266,10 +2266,8 @@ update(ChromeDebuggerActor.prototype, { */ globalManager: { findGlobals: function CDA_findGlobals() { - // Fetch the list of globals from the debugger. - for (let g of this.dbg.findAllGlobals()) { - this.addDebuggee(g); - } + // Add every global known to the debugger as debuggee. + this.dbg.addAllGlobalsAsDebuggees(); }, /**