From 0538cd57e29f9edd2cd10cb3e8b5e53cbcf16ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Ziad=C3=A9?= Date: Tue, 10 Jul 2018 10:06:41 +0200 Subject: [PATCH] Bug 1472668 - Don't reset performance counters when reading their values - r=baku We're changing the counters behavior since they are not notifications anymore. In the new behavior they don't get reset when they are retrieved, so we can have several consumers via the promise. If the values overflow, we let the wrapping occur (unsigned values). MozReview-Commit-ID: 1adkszScYo4 --HG-- extra : rebase_source : cd554ad4cfa643b09f75bb07e38b5d35e08cf470 --- dom/base/DocGroup.cpp | 2 -- .../browser_test_performance_metrics.js | 27 ++++++++++++------- dom/workers/WorkerDebugger.cpp | 1 - xpcom/tests/gtest/TestThreadMetrics.cpp | 12 --------- xpcom/threads/PerformanceCounter.cpp | 11 -------- xpcom/threads/PerformanceCounter.h | 9 +++---- 6 files changed, 22 insertions(+), 40 deletions(-) diff --git a/dom/base/DocGroup.cpp b/dom/base/DocGroup.cpp index b263da0ec7a6..dfad348f3717 100644 --- a/dom/base/DocGroup.cpp +++ b/dom/base/DocGroup.cpp @@ -129,8 +129,6 @@ DocGroup::ReportPerformanceInfo() } } - // setting back all counters to zero - mPerformanceCounter->ResetPerformanceCounters(); return PerformanceInfo(host, pid, pwid, duration, false, isTopLevel, items); } diff --git a/dom/tests/browser/browser_test_performance_metrics.js b/dom/tests/browser/browser_test_performance_metrics.js index b70733b6a60a..dc577ebd2c79 100644 --- a/dom/tests/browser/browser_test_performance_metrics.js +++ b/dom/tests/browser/browser_test_performance_metrics.js @@ -40,20 +40,16 @@ function postMessageToWorker(tab, message) { } add_task(async function test() { - SpecialPowers.setBoolPref('dom.performance.enable_scheduler_timing', true); + SpecialPowers.setBoolPref("dom.performance.enable_scheduler_timing", true); waitForExplicitFinish(); // Load 3 pages and wait. The 3rd one has a worker let page1 = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, opening: 'about:about', forceNewProcess: false + gBrowser, opening: "about:about", forceNewProcess: false }); let page2 = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, opening: 'about:memory', forceNewProcess: false - }); - - let page3 = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, opening: "about:performance", forceNewProcess: true + gBrowser, opening: "about:memory", forceNewProcess: false }); // load a 4th tab with a worker @@ -108,10 +104,23 @@ add_task(async function test() { Assert.ok(parentProcessEvent, "parent process sent back some events"); Assert.ok(isTopLevel, "example.com as a top level window"); Assert.ok(aboutMemoryFound, "about:memory"); + + // Doing a second call, we shoud get bigger values + let previousWorkerDuration = workerDuration; + let previousWorkerTotal = workerTotal; + let previousDuration = duration; + let previousTotal = total; + + results = await ChromeUtils.requestPerformanceMetrics(); + exploreResults(results); + + Assert.ok(workerDuration > previousWorkerDuration, "Worker duration should be positive"); + Assert.ok(workerTotal > previousWorkerTotal, "Worker count should be positive"); + Assert.ok(duration > previousDuration, "Duration should be positive"); + Assert.ok(total > previousTotal, "Should get a positive count"); }); BrowserTestUtils.removeTab(page1); BrowserTestUtils.removeTab(page2); - BrowserTestUtils.removeTab(page3); - SpecialPowers.clearUserPref('dom.performance.enable_scheduler_timing'); + SpecialPowers.clearUserPref("dom.performance.enable_scheduler_timing"); }); diff --git a/dom/workers/WorkerDebugger.cpp b/dom/workers/WorkerDebugger.cpp index c33d59ec6e37..4cc51bb92255 100644 --- a/dom/workers/WorkerDebugger.cpp +++ b/dom/workers/WorkerDebugger.cpp @@ -517,7 +517,6 @@ WorkerDebugger::ReportPerformanceInfo() return PerformanceInfo(uri->GetSpecOrDefault(), pid, pwid, duration, true, isTopLevel, items); } - perf->ResetPerformanceCounters(); } return PerformanceInfo(uri->GetSpecOrDefault(), pid, pwid, duration, diff --git a/xpcom/tests/gtest/TestThreadMetrics.cpp b/xpcom/tests/gtest/TestThreadMetrics.cpp index 1499504271be..22add0e8a06f 100644 --- a/xpcom/tests/gtest/TestThreadMetrics.cpp +++ b/xpcom/tests/gtest/TestThreadMetrics.cpp @@ -106,19 +106,7 @@ protected: mDispatchCount = (uint32_t)TaskCategory::Other + 1; } - void resetCounters() { - // let's reset the counters - mCounter->ResetPerformanceCounters(); - for (uint32_t i = 0; i < mDispatchCount; i++) { - ASSERT_EQ(mCounter->GetDispatchCounter()[i], 0u); - } - ASSERT_EQ(mCounter->GetExecutionDuration(), 0u); - } - virtual void TearDown() { - // let's reset the counters - resetCounters(); - // and remove the document from the doc group (actually, a nullptr) mDocGroup->RemoveDocument(nullptr); mDocGroup = nullptr; diff --git a/xpcom/threads/PerformanceCounter.cpp b/xpcom/threads/PerformanceCounter.cpp index 1cf74f141ec8..a15bbfe9a4e3 100644 --- a/xpcom/threads/PerformanceCounter.cpp +++ b/xpcom/threads/PerformanceCounter.cpp @@ -22,7 +22,6 @@ PerformanceCounter::PerformanceCounter(const nsACString& aName) mDispatchCounter(), mName(aName) { - ResetPerformanceCounters(); } void @@ -63,13 +62,3 @@ PerformanceCounter::GetDispatchCount(DispatchCategory aCategory) { return mDispatchCounter[aCategory.GetValue()]; } - -void -PerformanceCounter::ResetPerformanceCounters() -{ - for (auto& cnt : mDispatchCounter) { - cnt = 0; - } - mExecutionDuration = 0; - mTotalDispatchCount = 0; -} diff --git a/xpcom/threads/PerformanceCounter.h b/xpcom/threads/PerformanceCounter.h index 6403e3b737ab..0d0e9231415d 100644 --- a/xpcom/threads/PerformanceCounter.h +++ b/xpcom/threads/PerformanceCounter.h @@ -80,12 +80,16 @@ public: * This is called everytime a runnable is dispatched. * * aCategory can be used to distinguish counts per TaskCategory + * + * Note that an overflow will simply reset the counter. */ void IncrementDispatchCounter(DispatchCategory aCategory); /** * This is called via nsThread::ProcessNextEvent to measure runnable * execution duration. + * + * Note that an overflow will simply reset the counter. */ void IncrementExecutionDuration(uint32_t aMicroseconds); @@ -109,11 +113,6 @@ public: */ uint64_t GetTotalDispatchCount(); - /** - * Reset all counters and execution duration. - */ - void ResetPerformanceCounters(); - private: ~PerformanceCounter() {}