Changeset 157746 in webkit

Timestamp:

Oct 21, 2013, 3:00:40 PM (11 years ago)

Author:

mark.lam@apple.com

Message:

Avoid JSC debugger overhead unless needed.
​https://bugs.webkit.org/show_bug.cgi?id=123084.

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

• If no breakpoints are set, we now avoid calling the debug hook callbacks.
• If no break on exception is set, we also avoid exception event debug callbacks.
• When we return from the ScriptDebugServer to the JSC::Debugger, we may no longer call the debug hook callbacks if not needed. Hence, the m_currentCallFrame pointer in the ScriptDebugServer may become stale. To avoid this issue, before returning, the ScriptDebugServer will clear its m_currentCallFrame if needsOpDebugCallbacks() is false.

• debugger/Debugger.cpp:

(JSC::Debugger::Debugger):
(JSC::Debugger::setNeedsExceptionCallbacks):
(JSC::Debugger::setShouldPause):
(JSC::Debugger::updateNumberOfBreakpoints):
(JSC::Debugger::updateNeedForOpDebugCallbacks):

• debugger/Debugger.h:
• interpreter/Interpreter.cpp:

(JSC::Interpreter::unwind):
(JSC::Interpreter::debug):

• jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_debug):

• jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_debug):

• llint/LLIntOffsetsExtractor.cpp:
• llint/LowLevelInterpreter.asm:

Source/WebCore:

No new tests.

• If no breakpoints are set, we now avoid calling the debug hook callbacks.
• If no break on exception is set, we also avoid exception event debug callbacks.
• When we return from the ScriptDebugServer to the JSC::Debugger, we may no longer call the debug hook callbacks if not needed. Hence, the m_currentCallFrame pointer in the ScriptDebugServer may become stale. To avoid this issue, before returning, the ScriptDebugServer will clear its m_currentCallFrame if needsOpDebugCallbacks() is false.

• bindings/js/ScriptDebugServer.cpp:

(WebCore::ScriptDebugServer::setBreakpoint):
(WebCore::ScriptDebugServer::removeBreakpoint):
(WebCore::ScriptDebugServer::clearBreakpoints):
(WebCore::ScriptDebugServer::setPauseOnExceptionsState):
(WebCore::ScriptDebugServer::setPauseOnNextStatement):
(WebCore::ScriptDebugServer::breakProgram):
(WebCore::ScriptDebugServer::stepIntoStatement):
(WebCore::ScriptDebugServer::dispatchDidContinue):
(WebCore::ScriptDebugServer::exception):
(WebCore::ScriptDebugServer::didReachBreakpoint):

• inspector/InspectorDebuggerAgent.cpp:

(WebCore::InspectorDebuggerAgent::reset):

Source/WebKit/mac:

• WebView/WebScriptDebugger.mm:

(WebScriptDebugger::WebScriptDebugger):

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/debugger/Debugger.cpp (modified) (3 diffs)
• JavaScriptCore/debugger/Debugger.h (modified) (2 diffs)
• JavaScriptCore/interpreter/Interpreter.cpp (modified) (2 diffs)
• JavaScriptCore/jit/JITOpcodes.cpp (modified) (2 diffs)
• JavaScriptCore/jit/JITOpcodes32_64.cpp (modified) (2 diffs)
• JavaScriptCore/llint/LLIntOffsetsExtractor.cpp (modified) (1 diff)
• JavaScriptCore/llint/LowLevelInterpreter.asm (modified) (1 diff)
• JavaScriptCore/runtime/JSGlobalObject.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/ScriptDebugServer.cpp (modified) (13 diffs)
• WebCore/inspector/InspectorDebuggerAgent.cpp (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebView/WebScriptDebugger.mm (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-10-20  Mark Lam  <mark.lam@apple.com>
+
+        Avoid JSC debugger overhead unless needed.
+        https://bugs.webkit.org/show_bug.cgi?id=123084.
+
+        Reviewed by Geoffrey Garen.
+
+        - If no breakpoints are set, we now avoid calling the debug hook callbacks.
+        - If no break on exception is set, we also avoid exception event debug callbacks.
+        - When we return from the ScriptDebugServer to the JSC::Debugger, we may no
+          longer call the debug hook callbacks if not needed. Hence, the m_currentCallFrame
+          pointer in the ScriptDebugServer may become stale. To avoid this issue, before
+          returning, the ScriptDebugServer will clear its m_currentCallFrame if
+          needsOpDebugCallbacks() is false.
+
+        * debugger/Debugger.cpp:
+        (JSC::Debugger::Debugger):
+        (JSC::Debugger::setNeedsExceptionCallbacks):
+        (JSC::Debugger::setShouldPause):
+        (JSC::Debugger::updateNumberOfBreakpoints):
+        (JSC::Debugger::updateNeedForOpDebugCallbacks):
+        * debugger/Debugger.h:
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::unwind):
+        (JSC::Interpreter::debug):
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_debug):
+        * jit/JITOpcodes32_64.cpp:
+        (JSC::JIT::emit_op_debug):
+        * llint/LLIntOffsetsExtractor.cpp:
+        * llint/LowLevelInterpreter.asm:
+
 2013-10-21  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/JavaScriptCore/debugger/Debugger.cpp

@@ -92 +92 @@
 namespace JSC {
 
+Debugger::Debugger()
+    : m_needsExceptionCallbacks(false)
+    , m_needsOpDebugCallbacks(false)
+    , m_shouldPause(false)
+    , m_numberOfBreakpoints(0)
+{
+}
+
 Debugger::~Debugger()
 {
@@ -113 +121 @@
 }
 
+void Debugger::setNeedsExceptionCallbacks(bool value)
+{
+    m_needsExceptionCallbacks = value;
+}
+
+void Debugger::setShouldPause(bool value)
+{
+    m_shouldPause = value;
+    updateNeedForOpDebugCallbacks();
+}
+
 void Debugger::recompileAllJSFunctions(VM* vm)
 {
@@ -128 +147 @@
 }
 
+void Debugger::updateNumberOfBreakpoints(int numberOfBreakpoints)
+{
+    ASSERT(numberOfBreakpoints >= 0);
+    m_numberOfBreakpoints = numberOfBreakpoints;
+    updateNeedForOpDebugCallbacks();
+}
+
+void Debugger::updateNeedForOpDebugCallbacks()
+{
+    m_needsOpDebugCallbacks = m_shouldPause || m_numberOfBreakpoints;
+}
+
 } // namespace JSC

trunk/Source/JavaScriptCore/debugger/Debugger.h

@@ -38 +38 @@
 class JS_EXPORT_PRIVATE Debugger {
 public:
+    Debugger();
     virtual ~Debugger();
+
+    bool needsOpDebugCallbacks() const { return m_needsOpDebugCallbacks; }
+    static ptrdiff_t needsOpDebugCallbacksOffset() { return OBJECT_OFFSETOF(Debugger, m_needsOpDebugCallbacks); }
+
+    bool shouldPause() const { return m_shouldPause; }
+    void setShouldPause(bool);
+
+    bool needsExceptionCallbacks() const { return m_needsExceptionCallbacks; }
+    void setNeedsExceptionCallbacks(bool);
+
+    void incNumberOfBreakpoints() { updateNumberOfBreakpoints(m_numberOfBreakpoints + 1); }
+    void decNumberOfBreakpoints()  { updateNumberOfBreakpoints(m_numberOfBreakpoints - 1); }
+    void updateNumberOfBreakpoints(int);
 
     void attach(JSGlobalObject*);
@@ -57 +71 @@
 
 private:
+    void updateNeedForOpDebugCallbacks();
+
     HashSet<JSGlobalObject*> m_globalObjects;
+
+    bool m_needsExceptionCallbacks;
+    bool m_needsOpDebugCallbacks;
+    bool m_shouldPause;
+    int m_numberOfBreakpoints;
+
+    friend class LLIntOffsetsExtractor;
 };
 

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -651 +651 @@
     ASSERT(!exceptionValue.isObject() || asObject(exceptionValue)->hasProperty(callFrame, callFrame->vm().propertyNames->stack));
 
-    if (Debugger* debugger = callFrame->dynamicGlobalObject()->debugger()) {
+    Debugger* debugger = callFrame->dynamicGlobalObject()->debugger();
+    if (debugger && debugger->needsExceptionCallbacks()) {
         // We need to clear the exception and the exception stack here in order to see if a new exception happens.
         // Afterwards, the values are put back to continue processing this error.
@@ -1252 +1253 @@
 {
     Debugger* debugger = callFrame->dynamicGlobalObject()->debugger();
-    if (!debugger)
+    if (!debugger || !debugger->needsOpDebugCallbacks())
         return;
 

trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

@@ -31 +31 @@
 #include "Arguments.h"
 #include "CopiedSpaceInlines.h"
+#include "Debugger.h"
 #include "Heap.h"
 #include "JITInlines.h"
@@ -728 +729 @@
     breakpoint();
 #else
+    JSGlobalObject* globalObject = codeBlock()->globalObject();
+    Debugger* debugger = globalObject->debugger();
+    char* debuggerAddress = reinterpret_cast<char*>(globalObject) + JSGlobalObject::debuggerOffset();
+    Jump noDebugger = branchTestPtr(Zero, AbsoluteAddress(debuggerAddress));
+    char* flagAddress = reinterpret_cast<char*>(debugger) + Debugger::needsOpDebugCallbacksOffset();
+    Jump skipDebugHook = branchTest8(Zero, AbsoluteAddress(flagAddress));
     callOperation(operationDebug, currentInstruction[1].u.operand);
+    skipDebugHook.link(this);
+    noDebugger.link(this);
 #endif
 }

trunk/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

@@ -32 +32 @@
 
 #include "CCallHelpers.h"
+#include "Debugger.h"
 #include "JITInlines.h"
 #include "JSArray.h"
@@ -1052 +1053 @@
     breakpoint();
 #else
+    JSGlobalObject* globalObject = codeBlock()->globalObject();
+    Debugger* debugger = globalObject->debugger();
+    char* debuggerAddress = reinterpret_cast<char*>(globalObject) + JSGlobalObject::debuggerOffset();
+    loadPtr(debuggerAddress, regT0);
+    Jump noDebugger = branchTestPtr(Zero, regT0);
+    char* flagAddress = reinterpret_cast<char*>(debugger) + Debugger::needsOpDebugCallbacksOffset();
+    Jump skipDebugHook = branchTest8(Zero, AbsoluteAddress(flagAddress));
     callOperation(operationDebug, currentInstruction[1].u.operand);
+    skipDebugHook.link(this);
+    noDebugger.link(this);
 #endif
 }

trunk/Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp

@@ -28 +28 @@
 #include "ArrayProfile.h"
 #include "CodeBlock.h"
+#include "Debugger.h"
 #include "Executable.h"
 #include "Heap.h"

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm

@@ -798 +798 @@
 _llint_op_debug:
     traceExecution()
+    loadp CodeBlock[cfr], t0
+    loadp CodeBlock::m_globalObject[t0], t0
+    loadp JSGlobalObject::m_debugger[t0], t0
+    btiz t0, .opDebugDone
+    loadb Debugger::m_needsOpDebugCallbacks[t0], t0
+    btbz t0, .opDebugDone
+
     callSlowPath(_llint_slow_path_debug)
+.opDebugDone:                    
     dispatch(2)
 

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

@@ -468 +468 @@
     Debugger* debugger() const { return m_debugger; }
     void setDebugger(Debugger* debugger) { m_debugger = debugger; }
+    static ptrdiff_t debuggerOffset() { return OBJECT_OFFSETOF(JSGlobalObject, m_debugger); }
 
     const GlobalObjectMethodTable* globalObjectMethodTable() const { return m_globalObjectMethodTable; }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-10-20  Mark Lam  <mark.lam@apple.com>
+
+        Avoid JSC debugger overhead unless needed.
+        https://bugs.webkit.org/show_bug.cgi?id=123084.
+
+        Reviewed by Geoffrey Garen.
+
+        No new tests.
+
+        - If no breakpoints are set, we now avoid calling the debug hook callbacks.
+        - If no break on exception is set, we also avoid exception event debug callbacks.
+        - When we return from the ScriptDebugServer to the JSC::Debugger, we may no
+          longer call the debug hook callbacks if not needed. Hence, the m_currentCallFrame
+          pointer in the ScriptDebugServer may become stale. To avoid this issue, before
+          returning, the ScriptDebugServer will clear its m_currentCallFrame if
+          needsOpDebugCallbacks() is false.
+
+        * bindings/js/ScriptDebugServer.cpp:
+        (WebCore::ScriptDebugServer::setBreakpoint):
+        (WebCore::ScriptDebugServer::removeBreakpoint):
+        (WebCore::ScriptDebugServer::clearBreakpoints):
+        (WebCore::ScriptDebugServer::setPauseOnExceptionsState):
+        (WebCore::ScriptDebugServer::setPauseOnNextStatement):
+        (WebCore::ScriptDebugServer::breakProgram):
+        (WebCore::ScriptDebugServer::stepIntoStatement):
+        (WebCore::ScriptDebugServer::dispatchDidContinue):
+        (WebCore::ScriptDebugServer::exception):
+        (WebCore::ScriptDebugServer::didReachBreakpoint):
+        * inspector/InspectorDebuggerAgent.cpp:
+        (WebCore::InspectorDebuggerAgent::reset):
+
 2013-10-21  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/bindings/js/ScriptDebugServer.cpp

@@ -116 +116 @@
     }
     breaksVector.append(scriptBreakpoint);
+    incNumberOfBreakpoints();
 
     *actualLineNumber = scriptBreakpoint.lineNumber;
@@ -151 +152 @@
         if (breaksVector.at(i).columnNumber == static_cast<int>(columnNumber)) {
             breaksVector.remove(i);
+            decNumberOfBreakpoints();
             break;
         }
@@ -246 +248 @@
 {
     m_sourceIdToBreakpoints.clear();
+    updateNumberOfBreakpoints(0);
 }
 
@@ -256 +259 @@
 {
     m_pauseOnExceptionsState = pause;
+    setNeedsExceptionCallbacks(pause != DontPauseOnExceptions);
 }
 
@@ -261 +265 @@
 {
     m_pauseOnNextStatement = pause;
+    if (pause)
+        setShouldPause(true);
 }
 
@@ -269 +275 @@
 
     m_pauseOnNextStatement = true;
+    setShouldPause(true);
     pauseIfNeeded(m_currentCallFrame);
 }
@@ -287 +294 @@
 
     m_pauseOnNextStatement = true;
+    setShouldPause(true);
     m_doneProcessingDebuggerEvents = true;
 }
@@ -354 +362 @@
 {
     listener->didContinue();
+    if (!m_pauseOnNextStatement && !m_pauseOnCallFrame) {
+        setShouldPause(false);
+        if (!needsOpDebugCallbacks())
+            m_currentCallFrame = 0;
+    }
 }
 
@@ -477 +490 @@
     updateCallFrame(callFrame);
     pauseIfNeeded(callFrame);
+    if (!needsOpDebugCallbacks())
+        m_currentCallFrame = 0;
 }
 
@@ -530 +545 @@
 void ScriptDebugServer::callEvent(CallFrame* callFrame)
 {
-    if (!m_paused) {
-        updateCallFrame(callFrame);
-        pauseIfNeeded(callFrame);
-    }
+    if (!m_paused)
+        updateCallFrameAndPauseIfNeeded(callFrame);
 }
 
@@ -565 +578 @@
         return;
 
-    if (m_pauseOnExceptionsState == PauseOnAllExceptions || (m_pauseOnExceptionsState == PauseOnUncaughtExceptions && !hasHandler))
+    if (m_pauseOnExceptionsState == PauseOnAllExceptions || (m_pauseOnExceptionsState == PauseOnUncaughtExceptions && !hasHandler)) {
         m_pauseOnNextStatement = true;
+        setShouldPause(true);
+    }
 
     updateCallFrameAndPauseIfNeeded(callFrame);
@@ -573 +588 @@
 void ScriptDebugServer::willExecuteProgram(CallFrame* callFrame)
 {
-    if (!m_paused) {
-        updateCallFrame(callFrame);
-        pauseIfNeeded(callFrame);
-    }
+    if (!m_paused)
+        updateCallFrameAndPauseIfNeeded(callFrame);
 }
 
@@ -603 +616 @@
 
     m_pauseOnNextStatement = true;
+    setShouldPause(true);
     updateCallFrameAndPauseIfNeeded(callFrame);
 }

trunk/Source/WebCore/inspector/InspectorDebuggerAgent.cpp

@@ -824 +824 @@
 void InspectorDebuggerAgent::reset()
 {
+    scriptDebugServer().clearBreakpoints();
     m_scripts.clear();
     m_breakpointIdToDebugServerBreakpointIds.clear();

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2013-10-20  Mark Lam  <mark.lam@apple.com>
+
+        Avoid JSC debugger overhead unless needed.
+        https://bugs.webkit.org/show_bug.cgi?id=123084.
+
+        Reviewed by Geoffrey Garen.
+
+        * WebView/WebScriptDebugger.mm:
+        (WebScriptDebugger::WebScriptDebugger):
+
 2013-10-19  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKit/mac/WebView/WebScriptDebugger.mm

@@ -77 +77 @@
 {
     attach(globalObject);
+    setNeedsExceptionCallbacks(true);
 }