Attachment 361954 Details for Bug 194564 – [PATCH] Proposed Fix

[patch] [PATCH] Proposed Fix

cpu-1.patch (text/plain), 33.56 KB, created by Joseph Pecoraro on 2019-02-13 16:09:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2019-02-13 16:09:03 PST

Size: 33.56 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5c9e53414cc..a30a1308440 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better categorize CPU usage per-thread / worker
>+        https://bugs.webkit.org/show_bug.cgi?id=194564
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/cpu-profiler/resources/busy-worker.js: Added.
>+        * inspector/cpu-profiler/worker-threads-expected.txt: Added.
>+        * inspector/cpu-profiler/worker-threads.html: Added.
>+        Test that WebKit receives per-Worker CPU usage numbers.
>+
> 2019-02-12  Alex Christensen  <achristensen@webkit.org>
> 
>         Remove setDefersLoading infrastructure from WebKit2
>diff --git a/LayoutTests/inspector/cpu-profiler/resources/busy-worker.js b/LayoutTests/inspector/cpu-profiler/resources/busy-worker.js
>new file mode 100644
>index 00000000000..e7ae03693b3
>--- /dev/null
>+++ b/LayoutTests/inspector/cpu-profiler/resources/busy-worker.js
>@@ -0,0 +1,2 @@
>+// Intentional infinite loop.
>+while (true);
>diff --git a/LayoutTests/inspector/cpu-profiler/worker-threads-expected.txt b/LayoutTests/inspector/cpu-profiler/worker-threads-expected.txt
>new file mode 100644
>index 00000000000..8c6e4b2ddb4
>--- /dev/null
>+++ b/LayoutTests/inspector/cpu-profiler/worker-threads-expected.txt
>@@ -0,0 +1,21 @@
>+Tests that CPUProfiler tracking events include per-Worker CPU usage.
>+
>+
>+== Running test suite: CPUProfiler.Worker
>+-- Running test case: CPUProfiler.Worker
>+CPUProfiler.trackingStart
>+PASS: Should have a timestamp when starting.
>+CPUProfiler.trackingUpdate
>+PASS: Should have an event object.
>+PASS: Event should have a timestamp.
>+PASS: Event should have a usage.
>+PASS: usage should be greater than or equal to zero.
>+PASS: Event should have workerThreads.
>+PASS: Event should have 2 worker threads.
>+PASS: Worker 1 usage should be greater than zero.
>+PASS: Worker 2 usage should be greater than zero.
>+PASS: Worker 1 usage should be less than 100.
>+PASS: Worker 2 usage should be less than 100.
>+PASS: Total usage should be greater than or equal to the sum of both worker threads.
>+CPUProfiler.trackingComplete
>+
>diff --git a/LayoutTests/inspector/cpu-profiler/worker-threads.html b/LayoutTests/inspector/cpu-profiler/worker-threads.html
>new file mode 100644
>index 00000000000..7d21cd82933
>--- /dev/null
>+++ b/LayoutTests/inspector/cpu-profiler/worker-threads.html
>@@ -0,0 +1,59 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../http/tests/inspector/resources/protocol-test.js"></script>
>+<script>
>+let worker1 = new Worker("resources/busy-worker.js");
>+let worker2 = new Worker("resources/busy-worker.js");
>+
>+function test()
>+{
>+    let suite = ProtocolTest.createAsyncSuite("CPUProfiler.Worker");
>+
>+    suite.addTestCase({
>+        name: "CPUProfiler.Worker",
>+        test(resolve, reject) {
>+            InspectorProtocol.awaitEvent({event: "CPUProfiler.trackingStart"}).then((messageObject) => {
>+                ProtocolTest.log("CPUProfiler.trackingStart");
>+                ProtocolTest.expectThat(typeof messageObject.params.timestamp === "number", "Should have a timestamp when starting.");
>+            });
>+
>+            // Skip one update to be safe. The Worker may not have existed for long.
>+            InspectorProtocol.awaitEvent({event: "CPUProfiler.trackingUpdate"}).then((messageObject) => {
>+                InspectorProtocol.awaitEvent({event: "CPUProfiler.trackingUpdate"}).then((messageObject) => {
>+                    ProtocolTest.log("CPUProfiler.trackingUpdate");
>+                    ProtocolTest.expectThat(typeof messageObject.params.event === "object", "Should have an event object.");
>+                    ProtocolTest.expectThat(typeof messageObject.params.event.timestamp === "number", "Event should have a timestamp.");
>+                    ProtocolTest.expectThat(typeof messageObject.params.event.usage === "number", "Event should have a usage.");
>+                    ProtocolTest.expectThat(messageObject.params.event.usage >= 0, "usage should be greater than or equal to zero.");
>+
>+                    ProtocolTest.expectThat(messageObject.params.event.workerThreads, "Event should have workerThreads.");
>+                    ProtocolTest.expectEqual(messageObject.params.event.workerThreads.length, 2, "Event should have 2 worker threads.");
>+                    ProtocolTest.expectGreaterThan(messageObject.params.event.workerThreads[0].usage, 0, "Worker 1 usage should be greater than zero.");
>+                    ProtocolTest.expectGreaterThan(messageObject.params.event.workerThreads[1].usage, 0, "Worker 2 usage should be greater than zero.");
>+                    ProtocolTest.expectLessThanOrEqual(messageObject.params.event.workerThreads[0].usage, 100, "Worker 1 usage should be less than 100.");
>+                    ProtocolTest.expectLessThanOrEqual(messageObject.params.event.workerThreads[1].usage, 100, "Worker 2 usage should be less than 100.");
>+
>+                    ProtocolTest.expectGreaterThanOrEqual(messageObject.params.event.usage, messageObject.params.event.workerThreads[0].usage + messageObject.params.event.workerThreads[1].usage, "Total usage should be greater than or equal to the sum of both worker threads.");
>+
>+                    InspectorProtocol.sendCommand("CPUProfiler.stopTracking", {});
>+                });
>+            });
>+
>+            InspectorProtocol.awaitEvent({event: "CPUProfiler.trackingComplete"}).then((messageObject) => {
>+                ProtocolTest.log("CPUProfiler.trackingComplete");
>+                resolve();
>+            });
>+
>+            InspectorProtocol.sendCommand("CPUProfiler.startTracking", {});
>+        }
>+    });
>+
>+    suite.runTestCasesAndFinish();
>+}
>+</script>
>+</head>
>+<body onload="runTest()">
>+<p>Tests that CPUProfiler tracking events include per-Worker CPU usage.</p>
>+</body>
>+</html>
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 3ce7a6f3bd4..e1fc23773ae 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better categorize CPU usage per-thread / worker
>+        https://bugs.webkit.org/show_bug.cgi?id=194564
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/protocol/CPUProfiler.json:
>+        Add additional properties per-Event.
>+
> 2019-02-12  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Unreviewed, fix -Wimplicit-fallthrough warning after r241140
>diff --git a/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json b/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json
>index 2db2b501610..f329272c147 100644
>--- a/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json
>+++ b/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json
>@@ -4,12 +4,24 @@
>     "featureGuard": "ENABLE(RESOURCE_USAGE)",
>     "availability": ["web"],
>     "types": [
>+        {
>+            "id": "WorkerCPUUsage",
>+            "description": "CPU usage for an individual Worker.",
>+            "type": "object",
>+            "properties": [
>+                { "name": "targetId", "type": "string" },
>+                { "name": "usage", "type": "number" }
>+            ]
>+        },
>         {
>             "id": "Event",
>             "type": "object",
>             "properties": [
>                 { "name": "timestamp", "type": "number" },
>-                { "name": "usage", "type": "number", "description": "Percent of total cpu usage. If there are multiple cores the usage may be greater than 100%." }
>+                { "name": "usage", "type": "number", "description": "Percent of total cpu usage. If there are multiple cores the usage may be greater than 100%." },
>+                { "name": "mainThreadUsage", "type": "number", "optional": true, "description": "Usage by only the main thread. This should not exceed 100%." },
>+                { "name": "webkitThreadUsage", "type": "number", "optional": true, "description": "Usage by WebKit specific threads. GC Threads, Compiler Threads, etc." },
>+                { "name": "workerThreads", "type": "array", "items": { "$ref": "WorkerCPUUsage" }, "optional": true, "description": "Usage per Web Worker thread." }
>             ]
>         }
>     ],
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 7e0e7de4d5e..690ac146e39 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better categorize CPU usage per-thread / worker
>+        https://bugs.webkit.org/show_bug.cgi?id=194564
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/Threading.h:
>+        * wtf/Threading.cpp:
>+        (WTF::Thread::allThreadsMutex):
>+        (WTF::Thread::create):
>+        (WTF::Thread::didExit):
>+        Add a set of all WTF::Thread created threads.
>+
> 2019-02-12  David Kilzer  <ddkilzer@apple.com>
> 
>         REGRESSION (r238955, r240494): Soft-linking optional Lookup.framework triggers release assertion when missing
>diff --git a/Source/WTF/wtf/Threading.cpp b/Source/WTF/wtf/Threading.cpp
>index 31e4ddc045a..87ced9bd0fe 100644
>--- a/Source/WTF/wtf/Threading.cpp
>+++ b/Source/WTF/wtf/Threading.cpp
>@@ -66,6 +66,18 @@ public:
> #endif
> };
> 
>+HashSet<Thread*>& Thread::allThreads(const LockHolder&)
>+{
>+    static NeverDestroyed<HashSet<Thread*>> allThreads;
>+    return allThreads;
>+}
>+
>+Lock& Thread::allThreadsMutex()
>+{
>+    static Lock mutex;
>+    return mutex;
>+}
>+
> const char* Thread::normalizeThreadName(const char* threadName)
> {
> #if HAVE(PTHREAD_SETNAME_NP)
>@@ -164,6 +176,11 @@ Ref<Thread> Thread::create(const char* name, Function<void()>&& entryPoint)
> #endif
>     }
> 
>+    {
>+        LockHolder lock(allThreadsMutex());
>+        allThreads(lock).add(&thread.get());
>+    }
>+
>     ASSERT(!thread->stack().isEmpty());
>     return thread;
> }
>@@ -184,6 +201,11 @@ static bool shouldRemoveThreadFromThreadGroup()
> 
> void Thread::didExit()
> {
>+    {
>+        LockHolder lock(allThreadsMutex());
>+        allThreads(lock).remove(this);
>+    }
>+
>     if (shouldRemoveThreadFromThreadGroup()) {
>         {
>             Vector<std::shared_ptr<ThreadGroup>> threadGroups;
>diff --git a/Source/WTF/wtf/Threading.h b/Source/WTF/wtf/Threading.h
>index 3a8a4a55d33..b8974c1641a 100644
>--- a/Source/WTF/wtf/Threading.h
>+++ b/Source/WTF/wtf/Threading.h
>@@ -36,6 +36,7 @@
> #include <wtf/Expected.h>
> #include <wtf/FastTLS.h>
> #include <wtf/Function.h>
>+#include <wtf/HashSet.h>
> #include <wtf/PlatformRegisters.h>
> #include <wtf/Ref.h>
> #include <wtf/RefPtr.h>
>@@ -90,6 +91,10 @@ public:
>     // Returns Thread object.
>     static Thread& current();
> 
>+    // Set of all WTF::Thread created threads.
>+    WTF_EXPORT_PRIVATE static HashSet<Thread*>& allThreads(const LockHolder&);
>+    WTF_EXPORT_PRIVATE static Lock& allThreadsMutex();
>+
> #if OS(WINDOWS)
>     // Returns ThreadIdentifier directly. It is useful if the user only cares about identity
>     // of threads. At that time, users should know that holding this ThreadIdentifier does not ensure
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8907311e9c6..f93f7c4ad2d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better categorize CPU usage per-thread / worker
>+        https://bugs.webkit.org/show_bug.cgi?id=194564
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: inspector/cpu-profiler/worker-threads.html
>+
>+        * workers/WorkerThread.cpp:
>+        (WebCore::WorkerThread::workerThreadsMutex):
>+        (WebCore::WorkerThread::workerThreadCount):
>+        (WebCore::WorkerThread::WorkerThread):
>+        (WebCore::WorkerThread::~WorkerThread):
>+        (WebCore::WorkerThread::workerThread):
>+        (WebCore::WorkerThread::releaseFastMallocFreeMemoryInAllThreads):
>+        * workers/WorkerThread.h:
>+        (WebCore::WorkerThread::identifier const):
>+        Expose the set of all WorkerThreads.
>+
>+        * inspector/agents/InspectorCPUProfilerAgent.cpp:
>+        (WebCore::InspectorCPUProfilerAgent::collectSample):
>+        Send inspector additional per-thread data.
>+
>+        * page/ResourceUsageData.h:
>+        (WebCore::WorkerCPUInfo::WorkerCPUInfo):
>+        * page/cocoa/ResourceUsageThreadCocoa.mm:
>+        (WebCore::ThreadInfo::ThreadInfo):
>+        (WebCore::threadInfos):
>+        (WebCore::ResourceUsageThread::platformCollectCPUData):
>+        (WebCore::threadSendRights): Deleted.
>+        (WebCore::cpuUsage): Deleted.
>+        Compute per-thread values on cocoa ports.
>+
>+        * page/linux/ResourceUsageThreadLinux.cpp:
>+        (WebCore::ResourceUsageThread::platformCollectCPUData):
>+        Stub per-thread values on linux ports.
>+
> 2019-02-12  Alex Christensen  <achristensen@webkit.org>
> 
>         Remove setDefersLoading infrastructure from WebKit2
>diff --git a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>index 8dab1afcc97..c8b34b39634 100644
>--- a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>@@ -87,6 +87,24 @@ void InspectorCPUProfilerAgent::collectSample(const ResourceUsageData& data)
>         .setUsage(data.cpuExcludingDebuggerThreads)
>         .release();
> 
>+    if (data.cpuMainThread)
>+        event->setMainThreadUsage(*data.cpuMainThread);
>+
>+    if (data.cpuWebKitThreads)
>+        event->setWebkitThreadUsage(*data.cpuWebKitThreads);
>+
>+    if (!data.cpuWorkerThreads.isEmpty()) {
>+        RefPtr<JSON::ArrayOf<Protocol::CPUProfiler::WorkerCPUUsage>> workerThreads = JSON::ArrayOf<Protocol::CPUProfiler::WorkerCPUUsage>::create();
>+        for (auto& worker : data.cpuWorkerThreads) {
>+            auto workerUsage = Protocol::CPUProfiler::WorkerCPUUsage::create()
>+                .setTargetId(worker.threadIdentifier)
>+                .setUsage(worker.cpu)
>+                .release();
>+            workerThreads->addItem(WTFMove(workerUsage));
>+        }
>+        event->setWorkerThreads(WTFMove(workerThreads));
>+    }
>+
>     m_frontendDispatcher->trackingUpdate(WTFMove(event));
> }
> 
>diff --git a/Source/WebCore/page/ResourceUsageData.h b/Source/WebCore/page/ResourceUsageData.h
>index 12be8719ea3..7333664f898 100644
>--- a/Source/WebCore/page/ResourceUsageData.h
>+++ b/Source/WebCore/page/ResourceUsageData.h
>@@ -29,6 +29,7 @@
> 
> #include <array>
> #include <wtf/MonotonicTime.h>
>+#include <wtf/Optional.h>
> 
> namespace WebCore {
> 
>@@ -71,11 +72,26 @@ struct MemoryCategoryInfo {
>     unsigned type { MemoryCategory::NumberOfCategories };
> };
> 
>+struct WorkerCPUInfo {
>+    WorkerCPUInfo(String id, float usage)
>+        : threadIdentifier(id)
>+        , cpu(usage)
>+    {
>+    }
>+
>+    String threadIdentifier;
>+    float cpu { 0 };
>+};
>+
> struct ResourceUsageData {
>-    constexpr ResourceUsageData() = default;
>+    ResourceUsageData() = default;
> 
>     float cpu { 0 };
>     float cpuExcludingDebuggerThreads { 0 };
>+    Optional<float> cpuMainThread;
>+    Optional<float> cpuWebKitThreads; // Excludes Main Thread, Worker Threads, and Debugger threads.
>+    Vector<WorkerCPUInfo> cpuWorkerThreads;
>+
>     size_t totalDirtySize { 0 };
>     size_t totalExternalSize { 0 };
>     std::array<MemoryCategoryInfo, MemoryCategory::NumberOfCategories> categories { {
>diff --git a/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm b/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
>index 8956172e710..406450f4194 100644
>--- a/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
>+++ b/Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(RESOURCE_USAGE)
> 
>+#include "WorkerThread.h"
> #include <JavaScriptCore/GCActivityCallback.h>
> #include <JavaScriptCore/Heap.h>
> #include <JavaScriptCore/SamplingProfiler.h>
>@@ -141,43 +142,6 @@ std::array<TagInfo, 256> pagesPerVMTag()
>     return tags;
> }
> 
>-static Vector<MachSendRight> threadSendRights()
>-{
>-    thread_array_t threadList = nullptr;
>-    mach_msg_type_number_t threadCount = 0;
>-    kern_return_t kr = task_threads(mach_task_self(), &threadList, &threadCount);
>-    if (kr != KERN_SUCCESS)
>-        return { };
>-
>-    Vector<MachSendRight> machThreads;
>-    for (mach_msg_type_number_t i = 0; i < threadCount; ++i)
>-        machThreads.append(MachSendRight::adopt(threadList[i]));
>-
>-    kr = vm_deallocate(mach_task_self(), (vm_offset_t)threadList, threadCount * sizeof(thread_t));
>-    ASSERT(kr == KERN_SUCCESS);
>-
>-    return machThreads;
>-}
>-
>-static float cpuUsage(Vector<MachSendRight>& machThreads)
>-{
>-    float usage = 0;
>-
>-    for (auto& machThread : machThreads) {
>-        thread_info_data_t threadInfo;
>-        mach_msg_type_number_t threadInfoCount = THREAD_INFO_MAX;
>-        auto kr = thread_info(machThread.sendRight(), THREAD_BASIC_INFO, static_cast<thread_info_t>(threadInfo), &threadInfoCount);
>-        if (kr != KERN_SUCCESS)
>-            return -1;
>-
>-        auto threadBasicInfo = reinterpret_cast<thread_basic_info_t>(threadInfo);
>-        if (!(threadBasicInfo->flags & TH_FLAGS_IDLE))
>-            usage += threadBasicInfo->cpu_usage / static_cast<float>(TH_USAGE_SCALE) * 100.0;
>-    }
>-
>-    return usage;
>-}
>-
> static unsigned categoryForVMTag(unsigned tag)
> {
>     switch (tag) {
>@@ -205,6 +169,72 @@ static unsigned categoryForVMTag(unsigned tag)
>     }
> }
> 
>+struct ThreadInfo {
>+    ThreadInfo(MachSendRight machSendRight, float cpuUsage, const String& name, const String& queueName)
>+        : sendRight(WTFMove(machSendRight))
>+        , usage(cpuUsage)
>+        , threadName(name)
>+        , dispatchQueueName(queueName)
>+    {
>+    }
>+
>+    MachSendRight sendRight;
>+    float usage { 0 };
>+    String threadName;
>+    String dispatchQueueName;
>+};
>+
>+static Vector<ThreadInfo> threadInfos()
>+{
>+    thread_array_t threadList = nullptr;
>+    mach_msg_type_number_t threadCount = 0;
>+    kern_return_t kr = task_threads(mach_task_self(), &threadList, &threadCount);
>+    if (kr != KERN_SUCCESS)
>+        return { };
>+
>+    Vector<ThreadInfo> infos;
>+    for (mach_msg_type_number_t i = 0; i < threadCount; ++i) {
>+        MachSendRight sendRight = MachSendRight::adopt(threadList[i]);
>+
>+        thread_info_data_t threadInfo;
>+        mach_msg_type_number_t threadInfoCount = THREAD_INFO_MAX;
>+        kr = thread_info(sendRight.sendRight(), THREAD_BASIC_INFO, reinterpret_cast<thread_info_t>(&threadInfo), &threadInfoCount);
>+        if (kr != KERN_SUCCESS)
>+            continue;
>+
>+        thread_identifier_info_data_t threadIdentifierInfo;
>+        mach_msg_type_number_t threadIdentifierInfoCount = THREAD_IDENTIFIER_INFO_COUNT;
>+        kr = thread_info(sendRight.sendRight(), THREAD_IDENTIFIER_INFO, reinterpret_cast<thread_info_t>(&threadIdentifierInfo), &threadIdentifierInfoCount);
>+        if (kr != KERN_SUCCESS)
>+            continue;
>+
>+        thread_extended_info_data_t threadExtendedInfo;
>+        mach_msg_type_number_t threadExtendedInfoCount = THREAD_EXTENDED_INFO_COUNT;
>+        kr = thread_info(sendRight.sendRight(), THREAD_EXTENDED_INFO, reinterpret_cast<thread_info_t>(&threadExtendedInfo), &threadExtendedInfoCount);
>+        if (kr != KERN_SUCCESS)
>+            continue;
>+
>+        float usage = 0;
>+        auto threadBasicInfo = reinterpret_cast<thread_basic_info_t>(threadInfo);
>+        if (!(threadBasicInfo->flags & TH_FLAGS_IDLE))
>+            usage = threadBasicInfo->cpu_usage / static_cast<float>(TH_USAGE_SCALE) * 100.0;
>+
>+        String threadName = String(threadExtendedInfo.pth_name);
>+        String dispatchQueueName;
>+        if (threadIdentifierInfo.dispatch_qaddr) {
>+            dispatch_queue_t queue = *reinterpret_cast<dispatch_queue_t*>(threadIdentifierInfo.dispatch_qaddr);
>+            dispatchQueueName = String(dispatch_queue_get_label(queue));
>+        }
>+
>+        infos.append(ThreadInfo { WTFMove(sendRight), usage, threadName, dispatchQueueName });
>+    }
>+
>+    kr = vm_deallocate(mach_task_self(), (vm_offset_t)threadList, threadCount * sizeof(thread_t));
>+    ASSERT(kr == KERN_SUCCESS);
>+
>+    return infos;
>+}
>+
> void ResourceUsageThread::platformSaveStateBeforeStarting()
> {
> #if ENABLE(SAMPLING_PROFILER)
>@@ -214,13 +244,40 @@ void ResourceUsageThread::platformSaveStateBeforeStarting()
> 
> void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& data)
> {
>-    Vector<MachSendRight> threads = threadSendRights();
>-    data.cpu = cpuUsage(threads);
>+    Vector<ThreadInfo> threads = threadInfos();
>+    if (threads.isEmpty()) {
>+        ASSERT_NOT_REACHED();
>+        return;
>+    }
>+
>+    // Main thread is always first.
>+    ASSERT(threads[0].dispatchQueueName == "com.apple.main-thread");
> 
>-    // Remove debugger threads.
>     mach_port_t resourceUsageMachThread = mach_thread_self();
>-    threads.removeAllMatching([&] (MachSendRight& thread) {
>-        mach_port_t machThread = thread.sendRight();
>+    mach_port_t mainThreadMachThread = threads[0].sendRight.sendRight();
>+
>+    HashSet<mach_port_t> knownWebKitThreads;
>+    {
>+        LockHolder lock(Thread::allThreadsMutex());
>+        for (auto* thread : Thread::allThreads(lock)) {
>+            mach_port_t machThread = thread->machThread();
>+            if (machThread != MACH_PORT_NULL)
>+                knownWebKitThreads.add(machThread);
>+        }
>+    }
>+
>+    HashMap<mach_port_t, String> knownWorkerThreads;
>+    {
>+        LockHolder lock(WorkerThread::workerThreadsMutex());
>+        for (auto* thread : WorkerThread::workerThreads(lock)) {
>+            mach_port_t machThread = thread->thread()->machThread();
>+            if (machThread != MACH_PORT_NULL)
>+                knownWorkerThreads.set(machThread, thread->identifier().isolatedCopy());
>+        }
>+    }
>+
>+    auto isDebuggerThread = [&](const ThreadInfo& thread) -> bool {
>+        mach_port_t machThread = thread.sendRight.sendRight();
>         if (machThread == resourceUsageMachThread)
>             return true;
> #if ENABLE(SAMPLING_PROFILER)
>@@ -228,9 +285,50 @@ void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& da
>             return true;
> #endif
>         return false;
>-    });
>+    };
> 
>-    data.cpuExcludingDebuggerThreads = cpuUsage(threads);
>+    auto isWebKitThread = [&](const ThreadInfo& thread) -> bool {
>+        mach_port_t machThread = thread.sendRight.sendRight();
>+        if (knownWebKitThreads.contains(machThread))
>+            return true;
>+
>+        // The bmalloc scavenger thread is below WTF. Detect it by its name.
>+        if (thread.threadName == "JavaScriptCore bmalloc scavenger")
>+            return true;
>+
>+        // WebKit uses many WorkQueues with common prefixes.
>+        if (thread.dispatchQueueName.startsWith("com.apple.IPC.")
>+            || thread.dispatchQueueName.startsWith("com.apple.WebKit.")
>+            || thread.dispatchQueueName.startsWith("org.webkit."))
>+            return true;
>+
>+        return false;
>+    };
>+
>+    auto isWorkerThread = [&](const ThreadInfo& thread) -> bool {
>+        mach_port_t machThread = thread.sendRight.sendRight();
>+        return knownWorkerThreads.contains(machThread);
>+    };
>+
>+    data.cpuMainThread = threads[0].usage;
>+    data.cpuWebKitThreads = 0;
>+
>+    for (auto& thread : threads) {
>+        data.cpu += thread.usage;
>+
>+        if (isDebuggerThread(thread))
>+            continue;
>+        data.cpuExcludingDebuggerThreads += thread.usage;
>+
>+        mach_port_t machThread = thread.sendRight.sendRight();
>+        if (machThread == mainThreadMachThread)
>+            continue;
>+
>+        if (isWorkerThread(thread))
>+            data.cpuWorkerThreads.append(WorkerCPUInfo { knownWorkerThreads.get(machThread), thread.usage });
>+        else if (isWebKitThread(thread))
>+            data.cpuWebKitThreads = *data.cpuWebKitThreads + thread.usage;
>+    }
> }
> 
> 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 3149ff87b69..e5b894828ec 100644
>--- a/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
>+++ b/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
>@@ -160,7 +160,10 @@ void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& da
> 
>     // FIXME: Exclude the ResourceUsage thread.
>     // FIXME: Exclude the SamplingProfiler thread.
>+    // FIXME: Identify usage of the main thread.
>     data.cpuExcludingDebuggerThreads = data.cpu;
>+    data.cpuMainThread = 0;
>+    data.cpuWebKitThreads = 0;
> }
> 
> void ResourceUsageThread::platformCollectMemoryData(JSC::VM* vm, ResourceUsageData& data)
>diff --git a/Source/WebCore/workers/WorkerThread.cpp b/Source/WebCore/workers/WorkerThread.cpp
>index 22ea488e2d5..1f756186a20 100644
>--- a/Source/WebCore/workers/WorkerThread.cpp
>+++ b/Source/WebCore/workers/WorkerThread.cpp
>@@ -53,20 +53,22 @@
> 
> namespace WebCore {
> 
>-static Lock threadSetMutex;
>-
>-static HashSet<WorkerThread*>& workerThreads()
>+HashSet<WorkerThread*>& WorkerThread::workerThreads(const LockHolder&)
> {
>     static NeverDestroyed<HashSet<WorkerThread*>> workerThreads;
>-
>     return workerThreads;
> }
> 
>+Lock& WorkerThread::workerThreadsMutex()
>+{
>+    static Lock mutex;
>+    return mutex;
>+}
>+
> unsigned WorkerThread::workerThreadCount()
> {
>-    std::lock_guard<Lock> lock(threadSetMutex);
>-
>-    return workerThreads().size();
>+    LockHolder lock(workerThreadsMutex());
>+    return workerThreads(lock).size();
> }
> 
> struct WorkerThreadStartupData {
>@@ -107,7 +109,8 @@ WorkerThreadStartupData::WorkerThreadStartupData(const URL& scriptURL, const Str
> }
> 
> WorkerThread::WorkerThread(const URL& scriptURL, const String& name, const String& identifier, const String& userAgent, bool isOnline, const String& sourceCode, WorkerLoaderProxy& workerLoaderProxy, WorkerDebuggerProxy& workerDebuggerProxy, WorkerReportingProxy& workerReportingProxy, WorkerThreadStartMode startMode, const ContentSecurityPolicyResponseHeaders& contentSecurityPolicyResponseHeaders, bool shouldBypassMainWorldContentSecurityPolicy, const SecurityOrigin& topOrigin, MonotonicTime timeOrigin, IDBClient::IDBConnectionProxy* connectionProxy, SocketProvider* socketProvider, JSC::RuntimeFlags runtimeFlags, PAL::SessionID sessionID)
>-    : m_workerLoaderProxy(workerLoaderProxy)
>+    : m_identifier(identifier.isolatedCopy())
>+    , m_workerLoaderProxy(workerLoaderProxy)
>     , m_workerDebuggerProxy(workerDebuggerProxy)
>     , m_workerReportingProxy(workerReportingProxy)
>     , m_runtimeFlags(runtimeFlags)
>@@ -121,17 +124,15 @@ WorkerThread::WorkerThread(const URL& scriptURL, const String& name, const Strin
>     UNUSED_PARAM(connectionProxy);
> #endif
> 
>-    std::lock_guard<Lock> lock(threadSetMutex);
>-
>-    workerThreads().add(this);
>+    LockHolder lock(workerThreadsMutex());
>+    workerThreads(lock).add(this);
> }
> 
> WorkerThread::~WorkerThread()
> {
>-    std::lock_guard<Lock> lock(threadSetMutex);
>-
>-    ASSERT(workerThreads().contains(this));
>-    workerThreads().remove(this);
>+    LockHolder lock(workerThreadsMutex());
>+    ASSERT(workerThreads(lock).contains(this));
>+    workerThreads(lock).remove(this);
> }
> 
> void WorkerThread::start(WTF::Function<void(const String&)>&& evaluateCallback)
>@@ -191,12 +192,12 @@ void WorkerThread::workerThread()
> 
>     String exceptionMessage;
>     scriptController->evaluate(ScriptSourceCode(m_startupData->m_sourceCode, URL(m_startupData->m_scriptURL)), &exceptionMessage);
>-    
>+
>     callOnMainThread([evaluateCallback = WTFMove(m_evaluateCallback), message = exceptionMessage.isolatedCopy()] {
>         if (evaluateCallback)
>             evaluateCallback(message);
>     });
>-    
>+
>     // Free the startup data to cause its member variable deref's happen on the worker's thread (since
>     // all ref/derefs of these objects are happening on the thread at this point). Note that
>     // WorkerThread::~WorkerThread happens on a different thread where it was created.
>@@ -307,9 +308,8 @@ void WorkerThread::stop(WTF::Function<void()>&& stoppedCallback)
> 
> void WorkerThread::releaseFastMallocFreeMemoryInAllThreads()
> {
>-    std::lock_guard<Lock> lock(threadSetMutex);
>-
>-    for (auto* workerThread : workerThreads()) {
>+    LockHolder lock(workerThreadsMutex());
>+    for (auto* workerThread : workerThreads(lock)) {
>         workerThread->runLoop().postTask([] (ScriptExecutionContext&) {
>             WTF::releaseFastMallocFreeMemory();
>         });
>diff --git a/Source/WebCore/workers/WorkerThread.h b/Source/WebCore/workers/WorkerThread.h
>index 3dc54f6c473..0955dfeef18 100644
>--- a/Source/WebCore/workers/WorkerThread.h
>+++ b/Source/WebCore/workers/WorkerThread.h
>@@ -62,6 +62,9 @@ class WorkerThread : public ThreadSafeRefCounted<WorkerThread> {
> public:
>     virtual ~WorkerThread();
> 
>+    static HashSet<WorkerThread*>& workerThreads(const LockHolder&);
>+    static Lock& workerThreadsMutex();
>+
>     WEBCORE_EXPORT void start(WTF::Function<void(const String&)>&& evaluateCallback);
>     void stop(WTF::Function<void()>&& terminatedCallback);
> 
>@@ -85,6 +88,8 @@ public:
>     
>     JSC::RuntimeFlags runtimeFlags() const { return m_runtimeFlags; }
> 
>+    String identifier() const { return m_identifier; }
>+
> protected:
>     WorkerThread(const URL&, const String& name, const String& identifier, const String& userAgent, bool isOnline, const String& sourceCode, WorkerLoaderProxy&, WorkerDebuggerProxy&, WorkerReportingProxy&, WorkerThreadStartMode, const ContentSecurityPolicyResponseHeaders&, bool shouldBypassMainWorldContentSecurityPolicy, const SecurityOrigin& topOrigin, MonotonicTime timeOrigin, IDBClient::IDBConnectionProxy*, SocketProvider*, JSC::RuntimeFlags, PAL::SessionID);
> 
>@@ -104,6 +109,7 @@ private:
>     virtual bool isServiceWorkerThread() const { return false; }
> 
>     RefPtr<Thread> m_thread;
>+    String m_identifier;
>     WorkerRunLoop m_runLoop;
>     WorkerLoaderProxy& m_workerLoaderProxy;
>     WorkerDebuggerProxy& m_workerDebuggerProxy;
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index f39f41f82a0..83b524b47ea 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better categorize CPU usage per-thread / worker
>+        https://bugs.webkit.org/show_bug.cgi?id=194564
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UserInterface/Controllers/TimelineManager.js:
>+        (WI.TimelineManager.prototype.cpuProfilerTrackingUpdated):
>+        * UserInterface/Models/CPUTimelineRecord.js:
>+        (WI.CPUTimelineRecord.prototype.get mainThreadUsage):
>+        (WI.CPUTimelineRecord.prototype.get webkitThreadUsage):
>+        (WI.CPUTimelineRecord.prototype.get workerThreadUsage):
>+        (WI.CPUTimelineRecord.prototype.get unknownThreadUsage):
>+        (WI.CPUTimelineRecord.prototype.get workers):
>+        Build a better record from the protocol events.
>+
> 2019-02-12  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Experimental setting for CPU Usage Timeline improvements
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js b/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js
>index 20b7854d5fa..62c1d1117a9 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js
>@@ -444,7 +444,7 @@ WI.TimelineManager = class TimelineManager extends WI.Object
>         if (!this._isCapturing)
>             return;
> 
>-        this._addRecord(new WI.CPUTimelineRecord(event.timestamp, event.usage));
>+        this._addRecord(new WI.CPUTimelineRecord(event));
>     }
> 
>     cpuProfilerTrackingCompleted()
>diff --git a/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js b/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js
>index f0002844120..331af11fa8a 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js
>@@ -25,20 +25,43 @@
> 
> WI.CPUTimelineRecord = class CPUTimelineRecord extends WI.TimelineRecord
> {
>-    constructor(timestamp, usage)
>+    constructor({timestamp, usage, mainThreadUsage, webkitThreadUsage, workerThreads})
>     {
>         super(WI.TimelineRecord.Type.CPU, timestamp, timestamp);
> 
>         console.assert(typeof timestamp === "number");
>         console.assert(typeof usage === "number");
>         console.assert(usage >= 0);
>+        console.assert(mainThreadUsage === undefined || typeof mainThreadUsage === "number");
>+        console.assert(webkitThreadUsage === undefined || typeof webkitThreadUsage === "number");
>+        console.assert(workerThreads === undefined || Array.isArray(workerThreads));
> 
>         this._timestamp = timestamp;
>         this._usage = usage;
>+        this._mainThreadUsage = mainThreadUsage || 0;
>+        this._webkitThreadUsage = webkitThreadUsage || 0;
>+        this._workers = workerThreads || [];
>+        this._workerThreadUsage = this._workers.reduce((total, worker) => total + worker.usage, 0);
>+
>+        // Possible accumulation of floating point imprecisions might make the values slide slightly below zero.
>+        this._unknownThreadUsage = this._usage - this._mainThreadUsage - this._workerThreadUsage - this._webkitThreadUsage;
>+        console.assert(this._unknownThreadUsage >= 0 || this._unknownThreadUsage >= -0.0001);
>+        if (this._unknownThreadUsage < 0)
>+            this._unknownThreadUsage = 0;
>+
>+        console.assert(this._mainThreadUsage >= 0);
>+        console.assert(this._workerThreadUsage >= 0);
>+        console.assert(this._webkitThreadUsage >= 0);
>+        console.assert(this._unknownThreadUsage >= 0);
>     }
> 
>     // Public
> 
>     get timestamp() { return this._timestamp; }
>     get usage() { return this._usage; }
>+    get mainThreadUsage() { return this._mainThreadUsage; }
>+    get webkitThreadUsage() { return this._webkitThreadUsage; }
>+    get workerThreadUsage() { return this._workerThreadUsage; }
>+    get unknownThreadUsage() { return this._unknownThreadUsage; }
>+    get workers() { return this._workers; }
> };

Actions: View | Formatted Diff | Diff

Attachments on bug 194564: 361865 | 361954 | 362194 | 362200 | 362201 | 362320