Web Inspector: Separate Debugger enable state from the debugger breakpoints enabled...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Mar 2016 22:43:06 +0000 (22:43 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Mar 2016 22:43:06 +0000 (22:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152193
<rdar://problem/23867520>

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

When all breakpoints are disabled, we can recompile all JS
code and remove the necessary debugging code that is emitted.
This allows for the code that is executing to be almost as fast
as it is with the debugger completely disabled. This is in preparation for:
https://bugs.webkit.org/show_bug.cgi?id=155809
which will introduce a high fidelity profiler. That profiler
could be built off the principle that breakpoints are disabled
when we're performing a high fidelity profile. Doing so, for example,
allows the sampling profiler to better measure the real performance
of the JS of a particular application.

* debugger/Debugger.cpp:
(JSC::Debugger::setBreakpointsActivated):
(JSC::Debugger::setPauseOnExceptionsState):
* debugger/Debugger.h:
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
* inspector/JSGlobalObjectScriptDebugServer.cpp:
(Inspector::JSGlobalObjectScriptDebugServer::attachDebugger):
(Inspector::JSGlobalObjectScriptDebugServer::detachDebugger):
* inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::InspectorDebuggerAgent::enable):
* runtime/Executable.cpp:
(JSC::ScriptExecutable::newCodeBlockFor):
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::createProgramCodeBlock):
(JSC::JSGlobalObject::createEvalCodeBlock):
(JSC::JSGlobalObject::createModuleProgramCodeBlock):
(JSC::JSGlobalObject::queueMicrotask):
(JSC::JSGlobalObject::hasDebugger):
(JSC::JSGlobalObject::hasInteractiveDebugger):
* runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::runtimeFlags):
(JSC::JSGlobalObject::hasDebugger): Deleted.

Source/WebCore:

No new tests because this is already tested by inspector tests.

* inspector/PageScriptDebugServer.cpp:
(WebCore::PageScriptDebugServer::attachDebugger):
(WebCore::PageScriptDebugServer::detachDebugger):

LayoutTests:

* inspector/script-profiler/event-type-API.html:
* inspector/script-profiler/event-type-Microtask.html:
* inspector/script-profiler/event-type-Other.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@198648 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/script-profiler/event-type-API.html
LayoutTests/inspector/script-profiler/event-type-Microtask.html
LayoutTests/inspector/script-profiler/event-type-Other.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/debugger/Debugger.cpp
Source/JavaScriptCore/debugger/Debugger.h
Source/JavaScriptCore/dfg/DFGGraph.cpp
Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.cpp
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp
Source/JavaScriptCore/runtime/Executable.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/WebCore/ChangeLog
Source/WebCore/inspector/PageScriptDebugServer.cpp

index 06d4ca4..071fcad 100644 (file)
@@ -1,3 +1,15 @@
+2016-03-24  Saam barati  <sbarati@apple.com>
+
+        Web Inspector: Separate Debugger enable state from the debugger breakpoints enabled state
+        https://bugs.webkit.org/show_bug.cgi?id=152193
+        <rdar://problem/23867520>
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/script-profiler/event-type-API.html:
+        * inspector/script-profiler/event-type-Microtask.html:
+        * inspector/script-profiler/event-type-Other.html:
+
 2016-03-24  Daniel Bates  <dabates@apple.com>
 
         Update expected results following <http://trac.webkit.org/changeset/198591>
index 03163b1..95a6b81 100644 (file)
@@ -36,7 +36,7 @@ function test()
         }
     });
 
-    // FIXME: <https://webkit.org/b/152193> Web Inspector: Separate Debugger enable state from being attached
+    // FIXME: <https://webkit.org/b/155851> Web Inspector: We should separate out attaching the debugger from the Debugger.enable event
     // Debugger should not need to be enabled for profiling to work.
     InspectorProtocol.sendCommand("Debugger.enable", {});
 
index fbbd8fe..737189c 100644 (file)
@@ -37,7 +37,7 @@ function test()
         }
     });
 
-    // FIXME: <https://webkit.org/b/152193> Web Inspector: Separate Debugger enable state from being attached
+    // FIXME: <https://webkit.org/b/155851> Web Inspector: We should separate out attaching the debugger from the Debugger.enable event
     // Debugger should not need to be enabled for profiling to work.
     InspectorProtocol.sendCommand("Debugger.enable", {});
 
index ed38ef9..2060860 100644 (file)
@@ -54,7 +54,7 @@ function test()
         }
     });
 
-    // FIXME: <https://webkit.org/b/152193> Web Inspector: Separate Debugger enable state from being attached
+    // FIXME: <https://webkit.org/b/155851> Web Inspector: We should separate out attaching the debugger from the Debugger.enable event
     // Debugger should not need to be enabled for profiling to work.
     InspectorProtocol.sendCommand("Debugger.enable", {});
 
index 5b6c4a7..0371b4a 100644 (file)
@@ -1,3 +1,46 @@
+2016-03-24  Saam barati  <sbarati@apple.com>
+
+        Web Inspector: Separate Debugger enable state from the debugger breakpoints enabled state
+        https://bugs.webkit.org/show_bug.cgi?id=152193
+        <rdar://problem/23867520>
+
+        Reviewed by Joseph Pecoraro.
+
+        When all breakpoints are disabled, we can recompile all JS
+        code and remove the necessary debugging code that is emitted.
+        This allows for the code that is executing to be almost as fast
+        as it is with the debugger completely disabled. This is in preparation for:
+        https://bugs.webkit.org/show_bug.cgi?id=155809
+        which will introduce a high fidelity profiler. That profiler
+        could be built off the principle that breakpoints are disabled
+        when we're performing a high fidelity profile. Doing so, for example,
+        allows the sampling profiler to better measure the real performance
+        of the JS of a particular application.
+
+        * debugger/Debugger.cpp:
+        (JSC::Debugger::setBreakpointsActivated):
+        (JSC::Debugger::setPauseOnExceptionsState):
+        * debugger/Debugger.h:
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::Graph):
+        * inspector/JSGlobalObjectScriptDebugServer.cpp:
+        (Inspector::JSGlobalObjectScriptDebugServer::attachDebugger):
+        (Inspector::JSGlobalObjectScriptDebugServer::detachDebugger):
+        * inspector/agents/InspectorDebuggerAgent.cpp:
+        (Inspector::InspectorDebuggerAgent::enable):
+        * runtime/Executable.cpp:
+        (JSC::ScriptExecutable::newCodeBlockFor):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::createProgramCodeBlock):
+        (JSC::JSGlobalObject::createEvalCodeBlock):
+        (JSC::JSGlobalObject::createModuleProgramCodeBlock):
+        (JSC::JSGlobalObject::queueMicrotask):
+        (JSC::JSGlobalObject::hasDebugger):
+        (JSC::JSGlobalObject::hasInteractiveDebugger):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::runtimeFlags):
+        (JSC::JSGlobalObject::hasDebugger): Deleted.
+
 2016-03-24  Michael Saboff  <msaboff@apple.com>
 
         Create private builtin helper advanceStringIndexUnicode() for use by RegExp builtins
index f50d54b..b8d7160 100644 (file)
@@ -528,7 +528,11 @@ void Debugger::clearDebuggerRequests(JSGlobalObject* globalObject)
 
 void Debugger::setBreakpointsActivated(bool activated)
 {
+    if (activated == m_breakpointsActivated)
+        return;
+
     m_breakpointsActivated = activated;
+    recompileAllJSFunctions();
 }
 
 void Debugger::setPauseOnExceptionsState(PauseOnExceptionsState pause)
index 2e91aaf..00ece4a 100644 (file)
@@ -62,7 +62,8 @@ public:
         return m_currentException;
     }
 
-    bool needsExceptionCallbacks() const { return m_pauseOnExceptionsState != DontPauseOnExceptions; }
+    bool needsExceptionCallbacks() const { return m_breakpointsActivated && m_pauseOnExceptionsState != DontPauseOnExceptions; }
+    bool isInteractivelyDebugging() const { return m_breakpointsActivated; }
 
     enum ReasonForDetach {
         TerminatingDebuggingSession,
@@ -75,7 +76,6 @@ public:
     BreakpointID setBreakpoint(Breakpoint, unsigned& actualLine, unsigned& actualColumn);
     void removeBreakpoint(BreakpointID);
     void clearBreakpoints();
-    void setBreakpointsActivated(bool);
     void activateBreakpoints() { setBreakpointsActivated(true); }
     void deactivateBreakpoints() { setBreakpointsActivated(false); }
 
@@ -199,6 +199,7 @@ private:
         BreakpointDisabled,
         BreakpointEnabled
     };
+    void setBreakpointsActivated(bool);
     void toggleBreakpoint(CodeBlock*, Breakpoint&, BreakpointState);
     void applyBreakpoints(CodeBlock*);
     void toggleBreakpoint(Breakpoint&, BreakpointState);
index 8a71cbd..d561d6a 100644 (file)
@@ -78,7 +78,7 @@ Graph::Graph(VM& vm, Plan& plan, LongLivedState& longLivedState)
 {
     ASSERT(m_profiledBlock);
     
-    m_hasDebuggerEnabled = m_profiledBlock->globalObject()->hasDebugger()
+    m_hasDebuggerEnabled = m_profiledBlock->globalObject()->hasInteractiveDebugger()
         || Options::forceDebuggerBytecodeGeneration();
 }
 
index e9a28c1..667438b 100644 (file)
@@ -43,7 +43,6 @@ JSGlobalObjectScriptDebugServer::JSGlobalObjectScriptDebugServer(JSGlobalObject&
 void JSGlobalObjectScriptDebugServer::attachDebugger()
 {
     attach(&m_globalObject);
-    recompileAllJSFunctions();
 }
 
 void JSGlobalObjectScriptDebugServer::detachDebugger(bool isBeingDestroyed)
index 83d7f3b..475de11 100644 (file)
@@ -88,7 +88,6 @@ void InspectorDebuggerAgent::enable()
     if (m_enabled)
         return;
 
-    m_scriptDebugServer.setBreakpointsActivated(true);
     m_scriptDebugServer.addListener(this);
 
     if (m_listener)
index 8a55026..a993dfa 100644 (file)
@@ -297,7 +297,7 @@ CodeBlock* ScriptExecutable::newCodeBlockFor(
     RELEASE_ASSERT(!executable->codeBlockFor(kind));
     JSGlobalObject* globalObject = scope->globalObject();
     ParserError error;
-    DebuggerMode debuggerMode = globalObject->hasDebugger() ? DebuggerOn : DebuggerOff;
+    DebuggerMode debuggerMode = globalObject->hasInteractiveDebugger() ? DebuggerOn : DebuggerOff;
     ProfilerMode profilerMode = globalObject->hasLegacyProfiler() ? ProfilerOn : ProfilerOff;
     UnlinkedFunctionCodeBlock* unlinkedCodeBlock = 
         executable->m_unlinkedExecutable->unlinkedCodeBlockFor(
index 3e4e32e..d9c023e 100644 (file)
@@ -1000,7 +1000,7 @@ UnlinkedProgramCodeBlock* JSGlobalObject::createProgramCodeBlock(CallFrame* call
 {
     ParserError error;
     JSParserStrictMode strictMode = executable->isStrictMode() ? JSParserStrictMode::Strict : JSParserStrictMode::NotStrict;
-    DebuggerMode debuggerMode = hasDebugger() ? DebuggerOn : DebuggerOff;
+    DebuggerMode debuggerMode = hasInteractiveDebugger() ? DebuggerOn : DebuggerOff;
     ProfilerMode profilerMode = hasLegacyProfiler() ? ProfilerOn : ProfilerOff;
     UnlinkedProgramCodeBlock* unlinkedCodeBlock = vm().codeCache()->getProgramCodeBlock(
         vm(), executable, executable->source(), JSParserBuiltinMode::NotBuiltin, strictMode, 
@@ -1021,7 +1021,7 @@ UnlinkedEvalCodeBlock* JSGlobalObject::createEvalCodeBlock(CallFrame* callFrame,
 {
     ParserError error;
     JSParserStrictMode strictMode = executable->isStrictMode() ? JSParserStrictMode::Strict : JSParserStrictMode::NotStrict;
-    DebuggerMode debuggerMode = hasDebugger() ? DebuggerOn : DebuggerOff;
+    DebuggerMode debuggerMode = hasInteractiveDebugger() ? DebuggerOn : DebuggerOff;
     ProfilerMode profilerMode = hasLegacyProfiler() ? ProfilerOn : ProfilerOff;
     UnlinkedEvalCodeBlock* unlinkedCodeBlock = vm().codeCache()->getEvalCodeBlock(
         vm(), executable, executable->source(), JSParserBuiltinMode::NotBuiltin, strictMode, thisTDZMode, isArrowFunctionContext, debuggerMode, profilerMode, error, variablesUnderTDZ);
@@ -1040,7 +1040,7 @@ UnlinkedEvalCodeBlock* JSGlobalObject::createEvalCodeBlock(CallFrame* callFrame,
 UnlinkedModuleProgramCodeBlock* JSGlobalObject::createModuleProgramCodeBlock(CallFrame* callFrame, ModuleProgramExecutable* executable)
 {
     ParserError error;
-    DebuggerMode debuggerMode = hasDebugger() ? DebuggerOn : DebuggerOff;
+    DebuggerMode debuggerMode = hasInteractiveDebugger() ? DebuggerOn : DebuggerOff;
     ProfilerMode profilerMode = hasLegacyProfiler() ? ProfilerOn : ProfilerOff;
     UnlinkedModuleProgramCodeBlock* unlinkedCodeBlock = vm().codeCache()->getModuleProgramCodeBlock(
         vm(), executable, executable->source(), JSParserBuiltinMode::NotBuiltin, debuggerMode, profilerMode, error);
@@ -1152,4 +1152,14 @@ void JSGlobalObject::queueMicrotask(PassRefPtr<Microtask> task)
     vm().queueMicrotask(this, task);
 }
 
+bool JSGlobalObject::hasDebugger() const
+{ 
+    return m_debugger;
+}
+
+bool JSGlobalObject::hasInteractiveDebugger() const 
+{ 
+    return m_debugger && m_debugger->isInteractivelyDebugging();
+}
+
 } // namespace JSC
index 7c712cd..dcaa9ba 100644 (file)
@@ -368,7 +368,8 @@ public:
 
     DECLARE_EXPORT_INFO;
 
-    bool hasDebugger() const { return m_debugger; }
+    bool hasDebugger() const;
+    bool hasInteractiveDebugger() const;
     bool hasLegacyProfiler() const;
     const RuntimeFlags& runtimeFlags() const { return m_runtimeFlags; }
 
index 4f1fb24..7821f21 100644 (file)
@@ -1,3 +1,17 @@
+2016-03-24  Saam barati  <sbarati@apple.com>
+
+        Web Inspector: Separate Debugger enable state from the debugger breakpoints enabled state
+        https://bugs.webkit.org/show_bug.cgi?id=152193
+        <rdar://problem/23867520>
+
+        Reviewed by Joseph Pecoraro.
+
+        No new tests because this is already tested by inspector tests.
+
+        * inspector/PageScriptDebugServer.cpp:
+        (WebCore::PageScriptDebugServer::attachDebugger):
+        (WebCore::PageScriptDebugServer::detachDebugger):
+
 2016-03-24  Jer Noble  <jer.noble@apple.com>
 
         [MSE] Make calling HTMLMediaElement.buffered less expensive
index cb64a07..6bac5cc 100644 (file)
@@ -60,7 +60,6 @@ PageScriptDebugServer::PageScriptDebugServer(Page& page)
 void PageScriptDebugServer::attachDebugger()
 {
     m_page.setDebugger(this);
-    recompileAllJSFunctions();
 }
 
 void PageScriptDebugServer::detachDebugger(bool isBeingDestroyed)