1474844 - Filter out empty categories

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, enhancement, P1)

Description

[Tarek Ziadé (:tarek)]

We're dropping empty categories + improvements on the tests

Comment 1

[Tarek Ziadé (:tarek)]

Attachment: Bug 1474844 - Filter out empty categories -

Filters out empty categories when ChromeUtils.requestPerformanceMetrics() is called.
This test also adds test coverage and uses the worker windowId when it has no linked
window.

Review commit: https://reviewboard.mozilla.org/r/256202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256202/

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

https://reviewboard.mozilla.org/r/256202/#review263076

Tell me more about this windowId == 0. Thanks.

::: dom/workers/WorkerDebugger.cpp:489
(Diff revision 1)
>  #else
>    uint32_t pid = getpid();
>  #endif
>    bool isTopLevel= false;
> -  uint64_t pwid = 0;
> +  uint64_t windowID = mWorkerPrivate->WindowID();
>    nsPIDOMWindowInner* win = mWorkerPrivate->GetWindow();

for nested worker, if you want to get the window, you must follow the chain of WorkerPrivate. Here an example: https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/console/Console.cpp#702-708

follow up?

::: toolkit/components/perfmonitoring/PerformanceMetricsCollector.cpp:63
(Diff revision 1)
>    }
>    MOZ_ASSERT(mPendingResults > 0);
>  
>    // Each PerformanceInfo is converted into a PerformanceInfoDictionary
>    for (const PerformanceInfo& result : aMetrics) {
> +    if (result.windowId() == 0 && result.host().IsEmpty()) {

Why are you excluding them?

windowId() == 0 could be a SharedWorker or a ServiceWorker.

Comment 3

[Tarek Ziadé (:tarek)]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

https://reviewboard.mozilla.org/r/256202/#review263076

> Why are you excluding them?
> 
> windowId() == 0 could be a SharedWorker or a ServiceWorker.

When windowId == 0 and host == "" there's not much we can do with the info in about:performance. since this information is used to identify them, that's why I am dropping them here.

What would be the best way to uniquely identify SharedWorker or ServiceWorker ?

Comment 4

[Tarek Ziadé (:tarek)]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256202/diff/1-2/

Comment 5

[Tarek Ziadé (:tarek)]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256202/diff/2-3/

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

https://reviewboard.mozilla.org/r/256202/#review263622

Comment 7

[Tarek Ziadé (:tarek)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b64203285bb6be2ebec37aaf64f78b98da25f07

Comment 8

[Tarek Ziadé (:tarek)]

Comment on attachment 8991251 [details]
Bug 1474844 - Filter out empty categories -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256202/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e01bc035fe38
Filter out empty categories - r=baku

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/e01bc035fe38