From a3fdf4760e56f485e2a5d1c81e18d1c55cd10115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Ziad=C3=A9?= Date: Fri, 27 Jul 2018 11:44:22 +0200 Subject: [PATCH] Bug 1477943 - Add a unique id per PerformanceCounter instance - r=baku,froydnj This new id is added in the PerformanceInfo data and helps consumers distinguish counters. MozReview-Commit-ID: 7kEmqJcVggM --HG-- extra : rebase_source : 40cca4c937f846db93ec1315036ad1bac04bc762 --- dom/base/DocGroup.cpp | 5 ++-- dom/chrome-webidl/ChromeUtils.webidl | 1 + dom/ipc/DOMTypes.ipdlh | 2 ++ .../browser_test_performance_metrics.js | 6 ++++ dom/workers/WorkerDebugger.cpp | 6 ++-- .../PerformanceMetricsCollector.cpp | 1 + xpcom/threads/PerformanceCounter.cpp | 30 +++++++++++++++++-- xpcom/threads/PerformanceCounter.h | 14 +++++++++ 8 files changed, 58 insertions(+), 7 deletions(-) diff --git a/dom/base/DocGroup.cpp b/dom/base/DocGroup.cpp index 69acdc69b9f1..12e370959921 100644 --- a/dom/base/DocGroup.cpp +++ b/dom/base/DocGroup.cpp @@ -126,11 +126,12 @@ DocGroup::ReportPerformanceInfo() CategoryDispatch item = CategoryDispatch(index, count); if (!items.AppendElement(item, fallible)) { NS_ERROR("Could not complete the operation"); - return PerformanceInfo(host, pid, windowID, duration, false, isTopLevel, items); + break; } } - return PerformanceInfo(host, pid, windowID, duration, false, isTopLevel, items); + return PerformanceInfo(host, pid, windowID, duration, mPerformanceCounter->GetID(), + false, isTopLevel, items); } nsresult diff --git a/dom/chrome-webidl/ChromeUtils.webidl b/dom/chrome-webidl/ChromeUtils.webidl index 8681a0c49e81..a89f4bccb2fe 100644 --- a/dom/chrome-webidl/ChromeUtils.webidl +++ b/dom/chrome-webidl/ChromeUtils.webidl @@ -372,6 +372,7 @@ dictionary PerformanceInfoDictionary { unsigned long pid = 0; unsigned long long windowId = 0; unsigned long long duration = 0; + unsigned long long counterId = 0; boolean isWorker = false; boolean isTopLevel = false; sequence items = []; diff --git a/dom/ipc/DOMTypes.ipdlh b/dom/ipc/DOMTypes.ipdlh index 19fd4be2af68..d4bf3e12499d 100644 --- a/dom/ipc/DOMTypes.ipdlh +++ b/dom/ipc/DOMTypes.ipdlh @@ -158,6 +158,8 @@ struct PerformanceInfo uint64_t windowId; // Execution time in microseconds uint64_t duration; + // Counter ID (unique across processes) + uint64_t counterId; // True if the data is collected in a worker bool isWorker; // True if the document window is the top window diff --git a/dom/tests/browser/perfmetrics/browser_test_performance_metrics.js b/dom/tests/browser/perfmetrics/browser_test_performance_metrics.js index c393551d6f93..285920f82ed5 100644 --- a/dom/tests/browser/perfmetrics/browser_test_performance_metrics.js +++ b/dom/tests/browser/perfmetrics/browser_test_performance_metrics.js @@ -71,9 +71,13 @@ add_task(async function test() { let subFrameIds = []; let topLevelIds = []; let sharedWorker = false; + let counterIds = []; function exploreResults(data) { for (let entry of data) { + if (!counterIds.includes(entry.pid + ":" + entry.counterId)) { + counterIds.push(entry.pid + ":" + entry.counterId); + } sharedWorker = entry.host.endsWith("shared_worker.js") || sharedWorker; Assert.ok(entry.host != "" || entry.windowId !=0, @@ -123,6 +127,8 @@ add_task(async function test() { Assert.ok(isTopLevel, "example.com as a top level window"); Assert.ok(aboutMemoryFound, "about:memory"); Assert.ok(sharedWorker, "We got some info from a shared worker"); + let numCounters = counterIds.length; + Assert.ok(numCounters > 10, "This test generated at least " + numCounters + " unique ounters"); // checking that subframes are not orphans for (let frameId of subFrameIds) { diff --git a/dom/workers/WorkerDebugger.cpp b/dom/workers/WorkerDebugger.cpp index 988531ff4ed4..8e66c34dd606 100644 --- a/dom/workers/WorkerDebugger.cpp +++ b/dom/workers/WorkerDebugger.cpp @@ -514,19 +514,21 @@ WorkerDebugger::ReportPerformanceInfo() FallibleTArray items; uint64_t duration = 0; uint16_t count = 0; + uint64_t perfId = 0; RefPtr perf = mWorkerPrivate->GetPerformanceCounter(); if (perf) { + perfId = perf->GetID(); count = perf->GetTotalDispatchCount(); duration = perf->GetExecutionDuration(); CategoryDispatch item = CategoryDispatch(DispatchCategory::Worker.GetValue(), count); if (!items.AppendElement(item, fallible)) { NS_ERROR("Could not complete the operation"); - return PerformanceInfo(url, pid, windowID, duration, true, isTopLevel, items); + return PerformanceInfo(url, pid, windowID, duration, perfId, true, isTopLevel, items); } } - return PerformanceInfo(url, pid, windowID, duration, true, isTopLevel, items); + return PerformanceInfo(url, pid, windowID, duration, perfId, true, isTopLevel, items); } } // dom namespace diff --git a/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp b/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp index 3ce2133c900a..e2107fae9bb7 100644 --- a/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp +++ b/toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp @@ -85,6 +85,7 @@ AggregatedResults::AppendResult(const nsTArray& aMetrics) data->mWindowId = result.windowId(); data->mHost.Assign(result.host()); data->mDuration = result.duration(); + data->mCounterId = result.counterId(); data->mIsWorker = result.isWorker(); data->mIsTopLevel = result.isTopLevel(); data->mItems = items; diff --git a/xpcom/threads/PerformanceCounter.cpp b/xpcom/threads/PerformanceCounter.cpp index a15bbfe9a4e3..6c2e21addb9c 100644 --- a/xpcom/threads/PerformanceCounter.cpp +++ b/xpcom/threads/PerformanceCounter.cpp @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "mozilla/Atomics.h" #include "mozilla/Logging.h" #include "mozilla/PerformanceCounter.h" @@ -13,6 +14,19 @@ static mozilla::LazyLogModule sPerformanceCounter("PerformanceCounter"); #endif #define LOG(args) MOZ_LOG(sPerformanceCounter, mozilla::LogLevel::Debug, args) +// Global counter used by PerformanceCounter CTOR via NextCounterID(). +static Atomic gNextCounterID(0); + +static uint64_t +NextCounterID() +{ + // This can return the same value on different processes but + // we're fine with this behavior because consumers can use a (pid, counter_id) + // tuple to make instances globally unique in a browser session. + return ++gNextCounterID; +} + + // this instance is the extension for the worker const DispatchCategory DispatchCategory::Worker = DispatchCategory((uint32_t)TaskCategory::Count); @@ -20,8 +34,10 @@ PerformanceCounter::PerformanceCounter(const nsACString& aName) : mExecutionDuration(0), mTotalDispatchCount(0), mDispatchCounter(), - mName(aName) + mName(aName), + mID(NextCounterID()) { + LOG(("PerformanceCounter created with ID %" PRIu64, mID)); } void @@ -29,14 +45,16 @@ PerformanceCounter::IncrementDispatchCounter(DispatchCategory aCategory) { mDispatchCounter[aCategory.GetValue()] += 1; mTotalDispatchCount += 1; - LOG(("[%s] Total dispatch %" PRIu64, mName.get(), uint64_t(mTotalDispatchCount))); + LOG(("[%s][%" PRIu64 "] Total dispatch %" PRIu64, mName.get(), + GetID(), uint64_t(mTotalDispatchCount))); } void PerformanceCounter::IncrementExecutionDuration(uint32_t aMicroseconds) { mExecutionDuration += aMicroseconds; - LOG(("[%s] Total duration %" PRIu64, mName.get(), uint64_t(mExecutionDuration))); + LOG(("[%s][%" PRIu64 "] Total duration %" PRIu64, mName.get(), + GetID(), uint64_t(mExecutionDuration))); } const DispatchCounter& @@ -62,3 +80,9 @@ PerformanceCounter::GetDispatchCount(DispatchCategory aCategory) { return mDispatchCounter[aCategory.GetValue()]; } + +uint64_t +PerformanceCounter::GetID() const +{ + return mID; +} diff --git a/xpcom/threads/PerformanceCounter.h b/xpcom/threads/PerformanceCounter.h index 0d0e9231415d..5074acdb8814 100644 --- a/xpcom/threads/PerformanceCounter.h +++ b/xpcom/threads/PerformanceCounter.h @@ -113,6 +113,19 @@ public: */ uint64_t GetTotalDispatchCount(); + /** + * Returns the unique id for the instance. + * + * Used to distinguish instances since the lifespan of + * a PerformanceCounter can be shorter than the + * host it's tracking. That leads to edge cases + * where a counter appears to have values that go + * backwards. Having this id let the consumers + * detect that they are dealing with a new counter + * when it happens. + */ + uint64_t GetID() const; + private: ~PerformanceCounter() {} @@ -120,6 +133,7 @@ private: Atomic mTotalDispatchCount; DispatchCounter mDispatchCounter; nsCString mName; + const uint64_t mID; }; } // namespace mozilla