Closed
Bug 1447768
Opened 6 years ago
Closed 6 years ago
Extend ChromeUtils.RequestPerformanceMetrics to the main process
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: tarek, Assigned: tarek)
References
Details
(Whiteboard: [perf-tools])
Attachments
(2 files)
ChromeUtils.RequestPerformanceMetrics is currently querying for performance counters to all content processes via IPDL. Let's extend it so it also gets the counters from the main process (non-e10s, about: pages etc)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → tarek
Assignee | ||
Comment 1•6 years ago
|
||
Andrea, this is my proposal for this change: 1. Move all code inside ContentParent::RecvAddPerformanceMetrics to a PerformanceRunnable 2. The PerformanceRunnable constructor converts PerformanceInfo into a nsIPerformanceMetricsData 3. The PerformanceRunnable calls NotifyObservers with that data when dispatched to the main thread 4. Move the code in ContentChild::RecvRequestPerformanceMetrics that collects PerformanceInfo from all tabs and workers in a dedicated enumerator function located in <???> called GetPerformanceCounters() 5. Use GetPerformanceCounters() in ContentChild::RecvRequestPerformanceMetrics and send them back via IPDL 6. in ChromeUtils.RequestPerformanceMetrics(), use GetPerformanceCounters() for the current process and dispatch a PerformanceRunnable with it (besides doing the ContentParent calls for content processes) I am not sure where to put GetPerformanceCounters() though...
Flags: needinfo?(amarchesini)
Updated•6 years ago
|
Whiteboard: [perf-tools]
Comment 2•6 years ago
|
||
I and Tarek discussed this bug on IRC. Canceling the NI.
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 3•6 years ago
|
||
To summarize Andrea's feedback: GetPerformanceCounters() can be in toolkit/component and orchestrate everything (and grow into code that calls other counters) ChromeUtils.RequestPerformanceMetrics() will call that and will be the API that can be used in JS code in about:energy or about:performance to ask for data via a timer,
Assignee | ||
Comment 4•6 years ago
|
||
notice that the runnable we're dispatching for grabbing the metrics could use a new TaskCategory::PerformanceCollector
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Priority: -- → P2
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4a1f8e04d9b92e03d4f8701090427d66fb6ed54
Assignee | ||
Comment 9•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=37a93e66e07177aead1a09e73ab033488486e7a3
Assignee | ||
Comment 10•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=020f8c03d6053d9a0ddf2e831dbe83130fd0bfe6
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8962685 [details] Bug 1447768 - part 1 - Add PerformanceUtils helpers - https://reviewboard.mozilla.org/r/231552/#review237432 ::: dom/ipc/ContentParent.cpp:3327 (Diff revision 2) > > mozilla::ipc::IPCResult > ContentParent::RecvAddPerformanceMetrics(const PerformanceInfo& aMetrics) > { > #ifndef RELEASE_OR_BETA > - // converting the data we get from a child as a notification > + RefPtr<PerformanceInfoRunnable> runnable = new PerformanceInfoRunnable(aMetrics); Why do you need a runnable here? You can dispatch things directly from here. Maybe you just need an helper method. ::: dom/ipc/ContentParent.cpp:3331 (Diff revision 2) > #ifndef RELEASE_OR_BETA > - // converting the data we get from a child as a notification > - if (aMetrics.items().IsEmpty()) { > + RefPtr<PerformanceInfoRunnable> runnable = new PerformanceInfoRunnable(aMetrics); > + nsresult rv = NS_DispatchToMainThread(runnable.forget()); > + if (NS_WARN_IF(NS_FAILED(rv))) { > - return IPC_OK(); > + return IPC_OK(); > } #endif return IPC_OK(); instead #else... ::: toolkit/components/perfmonitoring/PerformanceCollector.h:14 (Diff revision 2) > +#include "nsIRunnable.h" > +#include "mozilla/dom/DocGroup.h" > +#include "mozilla/dom/DOMTypes.h" > +#include "mozilla/dom/WorkerDebuggerManager.h" > + > + put this class into mozilla namespace. ::: toolkit/components/perfmonitoring/PerformanceCollector.h:26 (Diff revision 2) > + * - WorkerDebugger > + */ > +class PerformanceInfoIterator final > +{ > +public: > + explicit PerformanceInfoIterator(); no explicit if there are no params. ::: toolkit/components/perfmonitoring/PerformanceCollector.h:45 (Diff revision 2) > + > +/** > + * Runnable that converts a PerformanceInfo into a nsIPerformanceMetricsData > + * and sends it through a notification > + */ > +class PerformanceInfoRunnable : public nsIRunnable class PerofmranceInfoRunnable final : public Runnable ::: toolkit/components/perfmonitoring/PerformanceCollector.h:50 (Diff revision 2) > +class PerformanceInfoRunnable : public nsIRunnable > +{ > +public: > + explicit PerformanceInfoRunnable(const mozilla::dom::PerformanceInfo& aMetrics); > + NS_DECL_ISUPPORTS > + NS_DECL_NSIRUNNABLE remove these 2 macros ::: toolkit/components/perfmonitoring/PerformanceCollector.h:51 (Diff revision 2) > +{ > +public: > + explicit PerformanceInfoRunnable(const mozilla::dom::PerformanceInfo& aMetrics); > + NS_DECL_ISUPPORTS > + NS_DECL_NSIRUNNABLE > + NS_IMETHOD Run() override; ::: toolkit/components/perfmonitoring/PerformanceCollector.h:53 (Diff revision 2) > + explicit PerformanceInfoRunnable(const mozilla::dom::PerformanceInfo& aMetrics); > + NS_DECL_ISUPPORTS > + NS_DECL_NSIRUNNABLE > + > +protected: > + virtual ~PerformanceInfoRunnable() {} = default ::: toolkit/components/perfmonitoring/PerformanceCollector.h:53 (Diff revision 2) > + explicit PerformanceInfoRunnable(const mozilla::dom::PerformanceInfo& aMetrics); > + NS_DECL_ISUPPORTS > + NS_DECL_NSIRUNNABLE > + > +protected: > + virtual ~PerformanceInfoRunnable() {} no virtual if final ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:49 (Diff revision 2) > + > +PerformanceInfo > +PerformanceInfoIterator::GetNext() > +{ > + MOZ_ASSERT(HasMore(), "iterating beyond end of array"); > + PerformanceInfo info; you are not using this. ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:53 (Diff revision 2) > + MOZ_ASSERT(HasMore(), "iterating beyond end of array"); > + PerformanceInfo info; > + > + // we send back worker counters first > + if (mCurrentIndex < mDebuggersLength) { > + WorkerDebugger* debugger = mWdm->GetDebuggerAt(mCurrentIndex++); This is tricky because if the some other code spins the event loop, a worker can go away. But I don't have a better approach to suggest. ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:56 (Diff revision 2) > + // we send back worker counters first > + if (mCurrentIndex < mDebuggersLength) { > + WorkerDebugger* debugger = mWdm->GetDebuggerAt(mCurrentIndex++); > + MOZ_ASSERT(debugger); > + return debugger->ReportPerformanceInfo(); > + } else { no else after a return. Just do: return ... } return ... ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:67 (Diff revision 2) > +/** > + * Class PerformanceInfoRunnable > + * > + */ > + > +PerformanceInfoRunnable::PerformanceInfoRunnable(const PerformanceInfo& aMetrics) if you make this class inheriting Runnable, here do: : Runnable("PerformanceInfoRunnable") , ... ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:74 (Diff revision 2) > + , mMetrics(aMetrics) > +{ > +} > + > + > +NS_IMPL_ISUPPORTS(PerformanceInfoRunnable, nsIRunnable) remove this. ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:76 (Diff revision 2) > +} > + > + > +NS_IMPL_ISUPPORTS(PerformanceInfoRunnable, nsIRunnable) > + > +nsresult NS_IMETHODIMP ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:92 (Diff revision 2) > + nsCOMPtr<nsIMutableArray> xpItems = do_CreateInstance(NS_ARRAY_CONTRACTID); > + if (NS_WARN_IF(!xpItems)) { > + return NS_ERROR_FAILURE; > + } > + > + for (uint32_t i = 0; i<mMetrics.items().Length(); i++) { i < mMetrics.. ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:97 (Diff revision 2) > + for (uint32_t i = 0; i<mMetrics.items().Length(); i++) { > + const CategoryDispatch& entry = mMetrics.items()[i]; > + nsCOMPtr<nsIPerformanceMetricsDispatchCategory> item = > + new PerformanceMetricsDispatchCategory(entry.category(), > + entry.count()); > + xpItems->AppendElement(item); this can fail. ::: toolkit/components/perfmonitoring/moz.build:42 (Diff revision 2) > ] > > FINAL_LIBRARY = 'xul' > + > +include('/ipc/chromium/chromium-config.mozbuild') > + no extra line
Attachment #8962685 -
Flags: review?(amarchesini)
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8962726 [details] Bug 1447768 - part 2 - Dispatch counters in the parent process - https://reviewboard.mozilla.org/r/231594/#review237444 Looks good! Just 2 comments to apply. ::: dom/base/ChromeUtils.cpp:676 (Diff revision 1) > + // collecting the current process counters > + RefPtr<PerformanceInfoIterator> iter = new PerformanceInfoIterator(); > + RefPtr<PerformanceInfoRunnable> runnable; > + nsresult rv; > + while (iter->HasMore()) { > + runnable = new PerformanceInfoRunnable(iter->GetNext()); Ok, I see why you want a runnable. I suggest to do something different. In the previous patch you are going to introduce an helper method. Here do a runnable function doing this: PerformanceInfoSomething matric = iter->GetNext(); NS_DispatchToMainThread( NS_NewRunnableFunction( "RequestPerformanceMetrics", [matric]() { YourHelperFunction(matric); })); ::: dom/workers/WorkerPrivate.cpp:5234 (Diff revision 1) > +} > > +PerformanceCounter* > +WorkerPrivate::GetPerformanceCounter() > +{ > + AssertIsOnMainThread(); This is wrong. You are using this method in WorkerThread::Dispatch that can be called on any thread. Remove this assertion.
Attachment #8962726 -
Flags: review?(amarchesini)
Assignee | ||
Comment 13•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=039d58a0b56917add742d3f75698d73eb8365258
Assignee | ||
Comment 14•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cbe622e394fcfe398c468dbf6ee20fda9ead258
Assignee | ||
Comment 15•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=daa8eba72f0d58e289f515d748790e5e0976ee94
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 18•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dab0144a9d7533f417090621c83b75d8cd8c1878
Comment hidden (mozreview-request) |
Assignee | ||
Comment 20•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f438f7f8ffeb068ab1c94f69a01a287771cc5f8
Comment hidden (mozreview-request) |
Comment 22•6 years ago
|
||
mozreview-review |
Comment on attachment 8962685 [details] Bug 1447768 - part 1 - Add PerformanceUtils helpers - https://reviewboard.mozilla.org/r/231552/#review237838 ::: dom/ipc/ContentChild.cpp:1391 (Diff revision 3) > > mozilla::ipc::IPCResult > ContentChild::RecvRequestPerformanceMetrics() > { > #ifndef RELEASE_OR_BETA > - // iterate on all WorkerDebugger > + RefPtr<PerformanceInfoIterator> iter = new PerformanceInfoIterator(); Instead of a iterator, you can have a collector. What about this approach: nsTArray<PerformanceInfo> info; PerformanceInfoUtils::CollectInfo(info); for (PerformanceInfo& i : info) { SendAddPerformanceMatrics(i); } ::: dom/ipc/ContentChild.cpp:1393 (Diff revision 3) > ContentChild::RecvRequestPerformanceMetrics() > { > #ifndef RELEASE_OR_BETA > - // iterate on all WorkerDebugger > - RefPtr<WorkerDebuggerManager> wdm = WorkerDebuggerManager::GetOrCreate(); > - if (NS_WARN_IF(!wdm)) { > + RefPtr<PerformanceInfoIterator> iter = new PerformanceInfoIterator(); > + while (iter->HasMore()) { > + SendAddPerformanceMetrics(iter->GetNext()); Plus, instead doing multiple send, what about if you do: SendAddPerformanceMatrics(info); <- the array. ::: dom/ipc/ContentParent.cpp:3327 (Diff revision 3) > > mozilla::ipc::IPCResult > ContentParent::RecvAddPerformanceMetrics(const PerformanceInfo& aMetrics) > { > #ifndef RELEASE_OR_BETA > - // converting the data we get from a child as a notification > + PerformanceInfoRunnable::NotifyPerformanceInfo(aMetrics); I would move this to PerformanceInfoUtils::NotifyPerformanceInfo(aMatrics); This would be an array. ::: toolkit/components/perfmonitoring/PerformanceCollector.h:26 (Diff revision 3) > + * > + * Counters collected from: > + * - DocGroup > + * - WorkerDebugger > + */ > +class PerformanceInfoIterator final No iterator if we use PerformanceUtils. ::: toolkit/components/perfmonitoring/PerformanceCollector.h:38 (Diff revision 3) > + > + > +private: > + ~PerformanceInfoIterator() = default; > + RefPtr<mozilla::dom::WorkerDebuggerManager> mWdm; > + mozilla::dom::DocGroup* mNextDocGroup; RefPtr<> ::: toolkit/components/perfmonitoring/PerformanceCollector.h:39 (Diff revision 3) > + > +private: > + ~PerformanceInfoIterator() = default; > + RefPtr<mozilla::dom::WorkerDebuggerManager> mWdm; > + mozilla::dom::DocGroup* mNextDocGroup; > + mozilla::dom::WorkerDebugger* mNextWorkerDebugger; RefPtr ::: toolkit/components/perfmonitoring/PerformanceCollector.cpp:22 (Diff revision 3) > +/** > + * Class PerformanceInfoIterator > + * > + */ > + > +PerformanceInfoIterator::PerformanceInfoIterator() : mHasMoreSet(false) , mCurrentIndex(0) { mWd = WorkerDebuggerManager::GetOrCreate(); } Just because mNextDocGroup is a RefPtr<> as nNextWorkerDebugger, you don't need to set them to nullptr.
Attachment #8962685 -
Flags: review?(amarchesini)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 25•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8dfe4076b20e8399d72b93640eea5e4e00baeda
Assignee | ||
Comment 26•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=596ebb1cea2c2cd5059db7b915f14dcca6602115
Assignee | ||
Comment 27•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf78b9008fb39c3915cf1cad10eb6bd78beead63
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 30•6 years ago
|
||
mozreview-review |
Comment on attachment 8962685 [details] Bug 1447768 - part 1 - Add PerformanceUtils helpers - https://reviewboard.mozilla.org/r/231552/#review239088 ::: dom/base/nsPerformanceMetrics.h:11 (Diff revision 5) > > #ifndef nsPerformanceMetrics_h___ > #define nsPerformanceMetrics_h___ > > #include "nsIPerformanceMetrics.h" > +#include "nsString.h" alphabetic order. ::: dom/ipc/ContentParent.cpp:3329 (Diff revision 5) > > mozilla::ipc::IPCResult > -ContentParent::RecvAddPerformanceMetrics(const PerformanceInfo& aMetrics) > +ContentParent::RecvAddPerformanceMetrics(nsTArray<PerformanceInfo>&& aMetrics) > { > #ifndef RELEASE_OR_BETA > - // converting the data we get from a child as a notification > + mozilla::NotifyPerformanceInfo(aMetrics); Unused << NS_WARN_IF(NS_FAILED(mozilla::NotifyPerformanceInfo(aMetrics))); ::: toolkit/components/perfmonitoring/PerformanceUtils.h:15 (Diff revision 5) > +#include "mozilla/dom/DocGroup.h" > +#include "mozilla/dom/DOMTypes.h" > +#include "mozilla/dom/WorkerDebugger.h" > +#include "mozilla/dom/WorkerDebuggerManager.h" > + > + What defines PerformanceInfo? ::: toolkit/components/perfmonitoring/PerformanceUtils.h:22 (Diff revision 5) > + > +/** > + * Collects all performance info in the current process > + * and adds then in the aMetrics arrey > + */ > +void CollectPerformanceInfo(nsTArray<mozilla::dom::PerformanceInfo>& aMetrics); no needs to have mozilla:: here. ::: toolkit/components/perfmonitoring/PerformanceUtils.h:28 (Diff revision 5) > + > +/** > + * Converts a PerformanceInfo array into a nsIPerformanceMetricsData and > + * sends a performance-metrics notification with it > + */ > +void NotifyPerformanceInfo(const nsTArray<mozilla::dom::PerformanceInfo>& aMetrics); remove mozilla:: ::: toolkit/components/perfmonitoring/PerformanceUtils.h:30 (Diff revision 5) > + * Converts a PerformanceInfo array into a nsIPerformanceMetricsData and > + * sends a performance-metrics notification with it > + */ > +void NotifyPerformanceInfo(const nsTArray<mozilla::dom::PerformanceInfo>& aMetrics); > + > + no extra lines here. ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:9 (Diff revision 5) > + * 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 "nsIMutableArray.h" > +#include "nsPerformanceMetrics.h" > +#include "mozilla/dom/WorkerDebugger.h" alphabetic order. ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:53 (Diff revision 5) > + * Helper to convert a PerformanceInfo into a nsIPerformanceMetricsData > + */ > +void > +NotifyPerformanceInfo(const nsTArray<PerformanceInfo>& aMetrics) > +{ > + nsCOMPtr<nsIMutableArray> subject = do_CreateInstance(NS_ARRAY_CONTRACTID); subject is a strange name. Call it 'array' or something else. ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:61 (Diff revision 5) > + } > + > + nsresult rv; > + > + // Each PerformanceInfo is converted into a nsIPerformanceMetricsData > + for (uint32_t i = 0; i < aMetrics.Length(); i++) { for (const PerformanceInfo& info : aMetrics) { ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:62 (Diff revision 5) > + > + nsresult rv; > + > + // Each PerformanceInfo is converted into a nsIPerformanceMetricsData > + for (uint32_t i = 0; i < aMetrics.Length(); i++) { > + PerformanceInfo info = aMetrics[i]; or: const PerformanceInfo& info = aMetrics[i]; this avoids an extra copy. ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:65 (Diff revision 5) > + // Each PerformanceInfo is converted into a nsIPerformanceMetricsData > + for (uint32_t i = 0; i < aMetrics.Length(); i++) { > + PerformanceInfo info = aMetrics[i]; > + nsCOMPtr<nsIMutableArray> items = do_CreateInstance(NS_ARRAY_CONTRACTID); > + if (NS_WARN_IF(!items)) { > + return; I would return the error, because this is probably a OOM. ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:68 (Diff revision 5) > + nsCOMPtr<nsIMutableArray> items = do_CreateInstance(NS_ARRAY_CONTRACTID); > + if (NS_WARN_IF(!items)) { > + return; > + } > + > + for (uint32_t itemIndex = 0; itemIndex < info.items().Length(); itemIndex++) { Probably you can do the same here: for (const CategoryDispatch& entry : info.items()) { ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:74 (Diff revision 5) > + const CategoryDispatch& entry = info.items()[itemIndex]; > + nsCOMPtr<nsIPerformanceMetricsDispatchCategory> item = > + new PerformanceMetricsDispatchCategory(entry.category(), > + entry.count()); > + rv = items->AppendElement(item); > + if (NS_WARN_IF(rv != NS_OK)) { if (NS_WARN_IF(NS_FAILED(rv))) { ... return ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:75 (Diff revision 5) > + nsCOMPtr<nsIPerformanceMetricsDispatchCategory> item = > + new PerformanceMetricsDispatchCategory(entry.category(), > + entry.count()); > + rv = items->AppendElement(item); > + if (NS_WARN_IF(rv != NS_OK)) { > + return; return rv ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:83 (Diff revision 5) > + nsCOMPtr<nsIPerformanceMetricsData> data; > + data = new PerformanceMetricsData(info.pid(), info.wid(), info.pwid(), > + info.host(), info.duration(), > + info.worker(), items); > + rv = subject->AppendElement(data); > + if (NS_WARN_IF(rv != NS_OK)) { NS_WARN_IF(NS_FAILED(rv)) ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:84 (Diff revision 5) > + data = new PerformanceMetricsData(info.pid(), info.wid(), info.pwid(), > + info.host(), info.duration(), > + info.worker(), items); > + rv = subject->AppendElement(data); > + if (NS_WARN_IF(rv != NS_OK)) { > + return; return rv ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:89 (Diff revision 5) > + return; > + } > + } > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + if (NS_WARN_IF(!obs)) { > + return; return NS_ERROR_FAILURE; ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:92 (Diff revision 5) > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + if (NS_WARN_IF(!obs)) { > + return; > + } > + rv = obs->NotifyObservers(subject, "performance-metrics", nullptr); > + if (NS_WARN_IF(rv != NS_OK)) { NS_WARN_IF(NS_FAILED(rv)) ::: toolkit/components/perfmonitoring/PerformanceUtils.cpp:93 (Diff revision 5) > + if (NS_WARN_IF(!obs)) { > + return; > + } > + rv = obs->NotifyObservers(subject, "performance-metrics", nullptr); > + if (NS_WARN_IF(rv != NS_OK)) { > + return; return rv
Attachment #8962685 -
Flags: review?(amarchesini) → review+
Comment 31•6 years ago
|
||
mozreview-review |
Comment on attachment 8962726 [details] Bug 1447768 - part 2 - Dispatch counters in the parent process - https://reviewboard.mozilla.org/r/231594/#review239090 ::: dom/base/ChromeUtils.cpp:681 (Diff revision 6) > + CollectPerformanceInfo(info); > + SystemGroup::Dispatch(TaskCategory::Performance, > + NS_NewRunnableFunction( > + "RequestPerformanceMetrics", > + [info]() { > + NotifyPerformanceInfo(info); indent this. or put it on the same line: [info]() { UNUSED << NS_WARN_IF(NS_FAILED(NotifyPerformanceInfo(info))); } ::: dom/tests/browser/browser_test_performance_metrics.js:16 (Diff revision 6) > add_task(async function test() { > if (!AppConstants.RELEASE_OR_BETA) { > SpecialPowers.setBoolPref('dom.performance.enable_scheduler_timing', true); > waitForExplicitFinish(); > > - await BrowserTestUtils.withNewTab({ gBrowser, url: "http://example.com" }, > + // Load 3 about: and wait about: pages, but example.com is not. ::: dom/tests/browser/browser_test_performance_metrics.js:26 (Diff revision 6) > + let about_about2 = await BrowserTestUtils.openNewForegroundTab({ > + gBrowser, opening: 'about:memory', forceNewProcess: false > + }); > + > + let about_about3 = await BrowserTestUtils.openNewForegroundTab({ > + gBrowser, opening: 'http://example.com', forceNewProcess: true this is not an about page. ::: dom/tests/browser/browser_test_performance_metrics.js:28 (Diff revision 6) > + }); > + > + let about_about3 = await BrowserTestUtils.openNewForegroundTab({ > + gBrowser, opening: 'http://example.com', forceNewProcess: true > + }); > + Do we have a test for workers? ::: dom/workers/WorkerDebugger.cpp:506 (Diff revision 6) > uint16_t count = perf->GetTotalDispatchCount(); > uint64_t duration = perf->GetExecutionDuration(); > RefPtr<nsIURI> uri = mWorkerPrivate->GetResolvedScriptURI(); > - CategoryDispatch item = CategoryDispatch(DispatchCategory::Worker.GetValue(), count); > FallibleTArray<CategoryDispatch> items; > + CategoryDispatch item = CategoryDispatch(DispatchCategory::Worker.GetValue(), count); why this?
Attachment #8962726 -
Flags: review?(amarchesini) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 34•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a47ac09041071e35e625839b15ed90e4cf98739
Comment 35•6 years ago
|
||
Pushed by tziade@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/71c9c86c3c8d part 1 - Add PerformanceUtils helpers - r=baku https://hg.mozilla.org/integration/autoland/rev/90eb45ff0a64 part 2 - Dispatch counters in the parent process - r=baku
Comment 36•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/71c9c86c3c8d https://hg.mozilla.org/mozilla-central/rev/90eb45ff0a64
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
Whiteboard: [perf-tools] → [perf-tools][geckoview:fxr]
Updated•6 years ago
|
Whiteboard: [perf-tools][geckoview:fxr] → [perf-tools],[geckoview:fxr]
Updated•6 years ago
|
Whiteboard: [perf-tools],[geckoview:fxr] → [perf-tools]
Comment 37•6 years ago
|
||
Sorry, editing wrong bug.
Comment 38•6 years ago
|
||
Hello Tarek: I see crashes such as https://crash-stats.mozilla.com/report/index/b83f127b-1cf2-42c1-a08e-d59790180726 that lead back to this bug - where should I add the signatures?
Flags: needinfo?(tarek)
Assignee | ||
Comment 39•6 years ago
|
||
Hey Marcia, I think we should create a new one - thanks!
Flags: needinfo?(tarek)
Assignee | ||
Comment 40•6 years ago
|
||
I've created https://bugzilla.mozilla.org/show_bug.cgi?id=1478684
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•