Web Inspector: Exclude Debugger Threads from CPU Usage values in Web Inspector

https://bugs.webkit.org/show_bug.cgi?id=193796
<rdar://problem/47532910>

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2019-01-25
Reviewed by Devin Rousso.

Source/JavaScriptCore:

* runtime/SamplingProfiler.cpp:
(JSC::SamplingProfiler::machThread):
* runtime/SamplingProfiler.h:
Expose the mach_port_t of the SamplingProfiler thread
so it can be tested against later.

Source/WebCore:

* page/ResourceUsageData.h:
* inspector/agents/InspectorCPUProfilerAgent.cpp:
(WebCore::InspectorCPUProfilerAgent::collectSample):
Show the CPU usage without debugger threads in the Web Inspector's timeline.

* page/ResourceUsageThread.h:
* page/cocoa/ResourceUsageThreadCocoa.mm:
(WebCore::ResourceUsageThread::platformSaveStateBeforeStarting):
For OS(DARWIN) ports, when starting to observe resource usage,
we grab the mach_port_t of SamplingProfiler on the main thread
in a thread safe way. For our purposes (Web Inspector timelines),
this will be good enough to identify the SamplingProfiler thread
during timeline recording. The SamplingProfiler thread won't change
during a timeline recording and recording start/stops will never
miss the SamplingProfiler changing.

(WebCore::filterThreads):
(WebCore::threadSendRights):
(WebCore::threadSendRightsExcludingDebuggerThreads):
(WebCore::cpuUsage):
(WebCore::ResourceUsageThread::platformCollectCPUData):
Calculate CPU usage twice, the second time excluding some threads.

* page/linux/ResourceUsageThreadLinux.cpp:
(WebCore::ResourceUsageThread::platformSaveStateBeforeStarting):
(WebCore::ResourceUsageThread::platformCollectCPUData):
Stubs for linux ports.

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 062ca8e..5a09c61 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,17 @@
+2019-01-25  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Exclude Debugger Threads from CPU Usage values in Web Inspector
+        https://bugs.webkit.org/show_bug.cgi?id=193796
+        <rdar://problem/47532910>
+
+        Reviewed by Devin Rousso.
+
+        * runtime/SamplingProfiler.cpp:
+        (JSC::SamplingProfiler::machThread):
+        * runtime/SamplingProfiler.h:
+        Expose the mach_port_t of the SamplingProfiler thread
+        so it can be tested against later.
+
 2019-01-25  Alex Christensen  <achristensen@webkit.org>
 
         Fix Windows build after r240511

diff --git a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
index f4b07d9..07df966 100644 (file)
--- a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
+++ b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp
@@ -1079,6 +1079,16 @@ void SamplingProfiler::reportTopBytecodes(PrintStream& out)
     }
 }
 
+#if OS(DARWIN)
+mach_port_t SamplingProfiler::machThread()
+{
+    if (!m_thread)
+        return MACH_PORT_NULL;
+
+    return m_thread->machThread();
+}
+#endif
+
 } // namespace JSC
 
 namespace WTF {

diff --git a/Source/JavaScriptCore/runtime/SamplingProfiler.h b/Source/JavaScriptCore/runtime/SamplingProfiler.h
index c79ae75..ffc9135 100644 (file)
--- a/Source/JavaScriptCore/runtime/SamplingProfiler.h
+++ b/Source/JavaScriptCore/runtime/SamplingProfiler.h
@@ -182,6 +182,10 @@ public:
     JS_EXPORT_PRIVATE void reportTopBytecodes();
     JS_EXPORT_PRIVATE void reportTopBytecodes(PrintStream&);
 
+#if OS(DARWIN)
+    JS_EXPORT_PRIVATE mach_port_t machThread();
+#endif
+
 private:
     void createThreadIfNecessary(const AbstractLocker&);
     void timerLoop();

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 3f1bf8d..027ded2 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2019-01-25  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Exclude Debugger Threads from CPU Usage values in Web Inspector
+        https://bugs.webkit.org/show_bug.cgi?id=193796
+        <rdar://problem/47532910>
+
+        Reviewed by Devin Rousso.
+
+        * page/ResourceUsageData.h:
+        * inspector/agents/InspectorCPUProfilerAgent.cpp:
+        (WebCore::InspectorCPUProfilerAgent::collectSample):
+        Show the CPU usage without debugger threads in the Web Inspector's timeline.
+
+        * page/ResourceUsageThread.h:
+        * page/cocoa/ResourceUsageThreadCocoa.mm:
+        (WebCore::ResourceUsageThread::platformSaveStateBeforeStarting):
+        For OS(DARWIN) ports, when starting to observe resource usage,
+        we grab the mach_port_t of SamplingProfiler on the main thread
+        in a thread safe way. For our purposes (Web Inspector timelines),
+        this will be good enough to identify the SamplingProfiler thread
+        during timeline recording. The SamplingProfiler thread won't change
+        during a timeline recording and recording start/stops will never
+        miss the SamplingProfiler changing.
+
+        (WebCore::filterThreads):
+        (WebCore::threadSendRights):
+        (WebCore::threadSendRightsExcludingDebuggerThreads):
+        (WebCore::cpuUsage):
+        (WebCore::ResourceUsageThread::platformCollectCPUData):
+        Calculate CPU usage twice, the second time excluding some threads.
+
+        * page/linux/ResourceUsageThreadLinux.cpp:
+        (WebCore::ResourceUsageThread::platformSaveStateBeforeStarting):
+        (WebCore::ResourceUsageThread::platformCollectCPUData):
+        Stubs for linux ports.
+
 2019-01-25  Zalan Bujtas  <zalan@apple.com>
 
         Remove FrameView::m_significantRenderedTextMilestonePending

diff --git a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
index 66248b9..8dab1af 100644 (file)
--- a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
+++ b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
@@ -84,7 +84,7 @@ void InspectorCPUProfilerAgent::collectSample(const ResourceUsageData& data)
 {
     auto event = Protocol::CPUProfiler::Event::create()
         .setTimestamp(m_environment.executionStopwatch()->elapsedTimeSince(data.timestamp).seconds())
-        .setUsage(data.cpu)
+        .setUsage(data.cpuExcludingDebuggerThreads)
         .release();
 
     m_frontendDispatcher->trackingUpdate(WTFMove(event));

diff --git a/Source/WebCore/page/ResourceUsageData.h b/Source/WebCore/page/ResourceUsageData.h
index ab977b5..12be871 100644 (file)
--- a/Source/WebCore/page/ResourceUsageData.h
+++ b/Source/WebCore/page/ResourceUsageData.h
@@ -75,6 +75,7 @@ struct ResourceUsageData {
     constexpr ResourceUsageData() = default;
 
     float cpu { 0 };
+    float cpuExcludingDebuggerThreads { 0 };
     size_t totalDirtySize { 0 };
     size_t totalExternalSize { 0 };
     std::array<MemoryCategoryInfo, MemoryCategory::NumberOfCategories> categories { {

diff --git a/Source/WebCore/page/ResourceUsageThread.h b/Source/WebCore/page/ResourceUsageThread.h
index 7dfb4a7..35ae3f0 100644 (file)
--- a/Source/WebCore/page/ResourceUsageThread.h
+++ b/Source/WebCore/page/ResourceUsageThread.h
@@ -37,6 +37,10 @@
 #include <wtf/Noncopyable.h>
 #include <wtf/Threading.h>
 
+#if OS(DARWIN)
+#include <mach/mach.h>
+#endif
+
 namespace JSC {
 class VM;
 }
@@ -70,6 +74,7 @@ private:
     void createThreadIfNeeded();
     void threadBody();
 
+    void platformSaveStateBeforeStarting();
     void platformCollectCPUData(JSC::VM*, ResourceUsageData&);
     void platformCollectMemoryData(JSC::VM*, ResourceUsageData&);
 
@@ -82,6 +87,11 @@ private:
     // Platforms may need to access some data from the common VM.
     // They should ensure their use of the VM is thread safe.
     JSC::VM* m_vm { nullptr };
+
+#if ENABLE(SAMPLING_PROFILER) && OS(DARWIN)
+    mach_port_t m_samplingProfilerMachThread { MACH_PORT_NULL };
+#endif
+
 };
 
 #if PLATFORM(COCOA)

diff --git a/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm b/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
index e644abe..d8e54be 100644 (file)
--- a/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
+++ b/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
@@ -30,6 +30,7 @@
 
 #include <JavaScriptCore/GCActivityCallback.h>
 #include <JavaScriptCore/Heap.h>
+#include <JavaScriptCore/SamplingProfiler.h>
 #include <JavaScriptCore/VM.h>
 #include <mach/mach.h>
 #include <mach/vm_statistics.h>
@@ -157,10 +158,8 @@ static Vector<MachSendRight> threadSendRights()
     return machThreads;
 }
 
-static float cpuUsage()
+static float cpuUsage(Vector<MachSendRight>& machThreads)
 {
-    auto machThreads = threadSendRights();
-
     float usage = 0;
 
     for (auto& machThread : machThreads) {
@@ -205,9 +204,32 @@ static unsigned categoryForVMTag(unsigned tag)
     }
 }
 
+void ResourceUsageThread::platformSaveStateBeforeStarting()
+{
+#if ENABLE(SAMPLING_PROFILER)
+    m_samplingProfilerMachThread = m_vm->samplingProfiler() ? m_vm->samplingProfiler()->machThread() : MACH_PORT_NULL;
+#endif
+}
+
 void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& data)
 {
-    data.cpu = cpuUsage();
+    Vector<MachSendRight> threads = threadSendRights();
+    data.cpu = cpuUsage(threads);
+
+    // Remove debugger threads.
+    mach_port_t resourceUsageMachThread = mach_thread_self();
+    threads.removeAllMatching([&] (MachSendRight& thread) {
+        mach_port_t machThread = thread.sendRight();
+        if (machThread == resourceUsageMachThread)
+            return true;
+#if ENABLE(SAMPLING_PROFILER)
+        if (machThread == m_samplingProfilerMachThread)
+            return true;
+#endif
+        return false;
+    });
+
+    data.cpuExcludingDebuggerThreads = cpuUsage(threads);
 }
 
 void ResourceUsageThread::platformCollectMemoryData(JSC::VM* vm, ResourceUsageData& data)

diff --git a/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp b/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
index f76f15a..3149ff8 100644 (file)
--- a/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
+++ b/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
@@ -150,9 +150,17 @@ static float cpuUsage()
     return clampTo<float>(usage, 0, 100);
 }
 
+void ResourceUsageThread::platformSaveStateBeforeStarting()
+{
+}
+
 void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& data)
 {
     data.cpu = cpuUsage();
+
+    // FIXME: Exclude the ResourceUsage thread.
+    // FIXME: Exclude the SamplingProfiler thread.
+    data.cpuExcludingDebuggerThreads = data.cpu;
 }
 
 void ResourceUsageThread::platformCollectMemoryData(JSC::VM* vm, ResourceUsageData& data)