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

[patch] [PATCH] Proposed Fix

cpu-3.patch (text/plain), 34.94 KB, created by Joseph Pecoraro on 2019-02-15 20:03:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2019-02-15 20:03:33 PST

Size: 34.94 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e8f5c36c80e..fa8607f7f23 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-02-15  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/threads-expected.txt: Added.
>+        * inspector/cpu-profiler/threads.html: Added.
>+        Test that WebKit receives per-Thread CPU usage numbers.
>+
> 2019-02-15  Dean Jackson  <dino@apple.com>
> 
>         Allow emulation of user gestures from Web Inspector console
>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/threads-expected.txt b/LayoutTests/inspector/cpu-profiler/threads-expected.txt
>new file mode 100644
>index 00000000000..549d21361c0
>--- /dev/null
>+++ b/LayoutTests/inspector/cpu-profiler/threads-expected.txt
>@@ -0,0 +1,22 @@
>+Tests that CPUProfiler tracking events include per-Thread CPU usage.
>+
>+
>+== Running test suite: CPUProfiler.Threads
>+-- Running test case: CPUProfiler.Threads
>+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 threads.
>+PASS: Every thread has between 0 and 100 usage.
>+PASS: Event should have 1 main thread.
>+PASS: Event should have 2 worker threads.
>+PASS: Event should have other worker threads.
>+PASS: Worker 1 usage should be greater than zero.
>+PASS: Worker 2 usage should be greater than zero.
>+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/threads.html b/LayoutTests/inspector/cpu-profiler/threads.html
>new file mode 100644
>index 00000000000..6c5eed54be3
>--- /dev/null
>+++ b/LayoutTests/inspector/cpu-profiler/threads.html
>@@ -0,0 +1,72 @@
>+<!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.Threads");
>+
>+    suite.addTestCase({
>+        name: "CPUProfiler.Threads",
>+        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 Workers may not have existed for long.
>+            InspectorProtocol.awaitEvent({event: "CPUProfiler.trackingUpdate"})
>+            .then(() => 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(Array.isArray(messageObject.params.event.threads), "Event should have threads.");
>+
>+                let main = [];
>+                let workers = [];
>+                let others = [];
>+                let threads = messageObject.params.event.threads;
>+                for (let thread of threads) {
>+                    if (thread.type === "main")
>+                        main.push(thread);
>+                    else if (thread.targetId)
>+                        workers.push(thread);
>+                    else
>+                        others.push(thread);
>+                }
>+
>+                ProtocolTest.expectThat(threads.every((x) => x.usage >= 0 && x.usage <= 100), "Every thread has between 0 and 100 usage.");
>+                ProtocolTest.expectEqual(main.length, 1, "Event should have 1 main thread.");
>+                ProtocolTest.expectEqual(workers.length, 2, "Event should have 2 worker threads.");
>+                ProtocolTest.expectThat(others.length > 0, "Event should have other worker threads.");
>+                ProtocolTest.expectGreaterThan(workers[0].usage, 0, "Worker 1 usage should be greater than zero.");
>+                ProtocolTest.expectGreaterThan(workers[1].usage, 0, "Worker 2 usage should be greater than zero.");
>+                ProtocolTest.expectGreaterThanOrEqual(messageObject.params.event.usage, workers[0].usage + workers[1].usage, "Total usage should be greater than or equal to the sum of both worker threads.");
>+
>+                InspectorProtocol.sendCommand("CPUProfiler.stopTracking", {});
>+            }).catch(reject);
>+
>+            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-Thread CPU usage.</p>
>+</body>
>+</html>
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 8049f0c376b..5ef0361797b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-02-15  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, and new per-Thread object info.
>+
> 2019-02-15  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Make builtin objects more lazily initialized under non-JIT mode
>diff --git a/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json b/Source/JavaScriptCore/inspector/protocol/CPUProfiler.json
>index 2db2b501610..cddf9caa7f0 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": "ThreadInfo",
>+            "description": "CPU usage for an individual thread.",
>+            "type": "object",
>+            "properties": [
>+                { "name": "name", "type": "string", "description": "Some thread identification information." },
>+                { "name": "usage", "type": "number", "description": "CPU usage for this thread. This should not exceed 100% for an individual thread." },
>+                { "name": "type", "type": "string", "enum": ["main", "webkit"], "optional": true, "description": "Type of thread. There should be a single main thread." },
>+                { "name": "targetId", "type": "string", "optional": true, "description": "A thread may be associated with a target, such as a Worker, in the process." }
>+            ]
>+        },
>         {
>             "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": "threads", "type": "array", "items": { "$ref": "ThreadInfo" }, "optional": true, "description": "Per-thread CPU usage information. Does not include the main thread." }
>             ]
>         }
>     ],
>diff --git a/Source/JavaScriptCore/inspector/scripts/codegen/generator.py b/Source/JavaScriptCore/inspector/scripts/codegen/generator.py
>index 86b0f07c799..1a85c248796 100755
>--- a/Source/JavaScriptCore/inspector/scripts/codegen/generator.py
>+++ b/Source/JavaScriptCore/inspector/scripts/codegen/generator.py
>@@ -53,6 +53,7 @@ _ENUM_IDENTIFIER_RENAME_MAP = {
>     'webmetal': 'WebMetal',  # Canvas.ContextType.webmetal
>     'bitmaprenderer': 'BitmapRenderer',  # Canvas.ContextType.bitmaprenderer
>     'webrtc': 'WebRTC',  # Console.ChannelSource.webrtc
>+    'webkit': 'WebKit',  # CPUProfiler.ThreadInfo.type
> }
> 
> # These objects are built manually by creating and setting JSON::Value instances.
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index af993227756..4ebb56b6305 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-02-15  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-15  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         [WTF] Add environment variable helpers
>diff --git a/Source/WTF/wtf/Threading.cpp b/Source/WTF/wtf/Threading.cpp
>index a54575c4e76..78643ba74ea 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 eee53ae6f3f..85cd3ed692f 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>
>@@ -99,6 +100,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 6625231d8f4..07ca1c00eba 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2019-02-15  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/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-15  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Make builtin objects more lazily initialized under non-JIT mode
>diff --git a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>index 8dab1afcc97..f30065c344b 100644
>--- a/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorCPUProfilerAgent.cpp
>@@ -80,6 +80,24 @@ void InspectorCPUProfilerAgent::stopTracking(ErrorString&)
>     m_frontendDispatcher->trackingComplete();
> }
> 
>+static Ref<Protocol::CPUProfiler::ThreadInfo> buildThreadInfo(const ThreadCPUInfo& thread)
>+{
>+    auto threadInfo = Protocol::CPUProfiler::ThreadInfo::create()
>+        .setName(thread.name)
>+        .setUsage(thread.cpu)
>+        .release();
>+
>+    if (thread.type == ThreadCPUInfo::Type::Main)
>+        threadInfo->setType(Protocol::CPUProfiler::ThreadInfo::Type::Main);
>+    else if (thread.type == ThreadCPUInfo::Type::WebKit)
>+        threadInfo->setType(Protocol::CPUProfiler::ThreadInfo::Type::WebKit);
>+
>+    if (!thread.identifier.isEmpty())
>+        threadInfo->setTargetId(thread.identifier);
>+
>+    return threadInfo;
>+}
>+
> void InspectorCPUProfilerAgent::collectSample(const ResourceUsageData& data)
> {
>     auto event = Protocol::CPUProfiler::Event::create()
>@@ -87,6 +105,13 @@ void InspectorCPUProfilerAgent::collectSample(const ResourceUsageData& data)
>         .setUsage(data.cpuExcludingDebuggerThreads)
>         .release();
> 
>+    if (!data.cpuThreads.isEmpty()) {
>+        RefPtr<JSON::ArrayOf<Protocol::CPUProfiler::ThreadInfo>> threads = JSON::ArrayOf<Protocol::CPUProfiler::ThreadInfo>::create();
>+        for (auto& threadInfo : data.cpuThreads)
>+            threads->addItem(buildThreadInfo(threadInfo));
>+        event->setThreads(WTFMove(threads));
>+    }
>+
>     m_frontendDispatcher->trackingUpdate(WTFMove(event));
> }
> 
>diff --git a/Source/WebCore/page/ResourceUsageData.h b/Source/WebCore/page/ResourceUsageData.h
>index 12be8719ea3..17af37168cd 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,34 @@ struct MemoryCategoryInfo {
>     unsigned type { MemoryCategory::NumberOfCategories };
> };
> 
>+struct ThreadCPUInfo {
>+    enum class Type : uint8_t {
>+        Unknown,
>+        Main,
>+        WebKit,
>+    };
>+
>+    ThreadCPUInfo(String threadName, String threadIdentifier, float threadUsage, Type threadType)
>+        : name(threadName)
>+        , identifier(threadIdentifier)
>+        , cpu(threadUsage)
>+        , type(threadType)
>+    {
>+    }
>+
>+    String name;
>+    String identifier;
>+    float cpu { 0 };
>+    Type type { ThreadCPUInfo::Type::Unknown };
>+};
>+
> struct ResourceUsageData {
>-    constexpr ResourceUsageData() = default;
>+    ResourceUsageData() = default;
> 
>     float cpu { 0 };
>     float cpuExcludingDebuggerThreads { 0 };
>+    Vector<ThreadCPUInfo> cpuThreads;
>+
>     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..2f1901276fc 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,44 @@ 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;
>+    };
>+
>+    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) {
>+            data.cpuThreads.append(ThreadCPUInfo { "Main Thread"_s, String(), thread.usage, ThreadCPUInfo::Type::Main});
>+            continue;
>+        }
>+
>+        String threadIdentifier = knownWorkerThreads.get(machThread);
>+        bool isWorkerThread = !threadIdentifier.isEmpty();
>+        ThreadCPUInfo::Type type = (isWorkerThread || isWebKitThread(thread)) ? ThreadCPUInfo::Type::WebKit : ThreadCPUInfo::Type::Unknown;
>+        data.cpuThreads.append(ThreadCPUInfo { thread.threadName, threadIdentifier, thread.usage, type });
>+    }
> }
> 
> 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..c4460afb61a 100644
>--- a/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
>+++ b/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp
>@@ -160,6 +160,7 @@ void ResourceUsageThread::platformCollectCPUData(JSC::VM*, ResourceUsageData& da
> 
>     // FIXME: Exclude the ResourceUsage thread.
>     // FIXME: Exclude the SamplingProfiler thread.
>+    // FIXME: Classify usage per thread.
>     data.cpuExcludingDebuggerThreads = data.cpu;
> }
> 
>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 5e3280fc75f..57692543bac 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,19 @@
>+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):
>+        Build a better record from the protocol events.
>+
> 2019-02-15  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Unreviewed, fix intentation.
>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..b4abb2d3f64 100644
>--- a/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js
>+++ b/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2016 Apple Inc. All rights reserved.
>+ * Copyright (C) 2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -25,20 +25,51 @@
> 
> WI.CPUTimelineRecord = class CPUTimelineRecord extends WI.TimelineRecord
> {
>-    constructor(timestamp, usage)
>+    constructor({timestamp, usage, threads})
>     {
>         super(WI.TimelineRecord.Type.CPU, timestamp, timestamp);
> 
>         console.assert(typeof timestamp === "number");
>         console.assert(typeof usage === "number");
>         console.assert(usage >= 0);
>+        console.assert(threads === undefined || Array.isArray(threads));
> 
>         this._timestamp = timestamp;
>         this._usage = usage;
>+
>+        threads = threads || [];
>+
>+        this._mainThreadUsage = 0;
>+        this._webkitThreadUsage = 0;
>+        this._workerThreadUsage = 0;
>+        this._unknownThreadUsage = 0;
>+
>+        for (let thread of threads) {
>+            if (thread.type === InspectorBackend.domains.CPUProfiler.ThreadInfoType.Main) {
>+                console.assert(!this._mainThreadUsage);
>+                this._mainThreadUsage += thread.usage;
>+                continue;
>+            }
>+
>+            if (thread.type === InspectorBackend.domains.CPUProfiler.ThreadInfoType.WebKit) {
>+                if (thread.targetId)
>+                    this._workerThreadUsage += thread.usage;
>+                else
>+                    this._webkitThreadUsage += thread.usage;
>+                continue;
>+            }
>+
>+            this._unknownThreadUsage += thread.usage;
>+        }
>     }
> 
>     // 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; }
> };

Flags:

hi: review+

Actions: View | Formatted Diff | Diff

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