1548540 - _sendPerformanceCounters is one of the common timer based wake ups in child processes

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

I don't know what _sendPerformanceCounters is used for, but it seems to run rather often in child process.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

this is definitely among the most common wake ups on the main thread on otherwise rather idle child process (say, about:blank)
Anything we could do here?

Comment 2

[Tarek Ziadé (:tarek)]

_sendPerformanceCounters is called when we display some metrics in about:performance - This is supposed to run only when about:performance is open in one tab, if you see calls outside this use case, it's a bug.

One option would be to pause it when about:performance doesn't have the focus (but maybe that's the case?) adding Florian who's doing the UI

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

It runs all the time, even without about:performance.

Comment 4

[Florian Quèze [:florian]]

(In reply to Tarek Ziadé (:tarek) from comment #2)

One option would be to pause it when about:performance doesn't have the focus (but maybe that's the case?) adding Florian who's doing the UI

When about:performance is in a currently invisible tab, we pause the UI refresh, but we keep collecting the data in the background because to display the energy impact column when the user switches back to about:performance, we need to compare the current data with the data from 2s before.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

To clarify, I see the function being called after startup (and seemingly ongoing forever) when the only tab there is and has been is about:blank.

Comment 6

[Tarek Ziadé (:tarek)]

That would be a bug then, it's not supposed to be called but by that UI, I will investigate when I am back from PTO (next monday) - if you want someone else to look earlier don't hesitate to reassign it

Comment 7

[Tarek Ziadé (:tarek)]

oh... I was confused by the API name (wrt to the C++ side), sorry!

We have JS API calls counters we use in about:performance, and they are incremented in each child on each call. Kris asked me to add this timer so we would avoid keeping them in memory on each child. That timer grabs them and aggregates them in the parent in a single structure.

The simplest thing to do is to reduce the number of call by chaging the default here : https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#5184

5 seconds sounds fine.

We can do a more sophisticated thing if you think it's worth it, like triggering the timer only if the counters have reach a certain size in the child.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

In the long run we should avoid using timers. We'll get way more processes and if each of them fire some timer, there will be quite some wake-ups.

But for now, I'll try increase that value.

Comment 9

[Tarek Ziadé (:tarek)]

Ok. I'll add a patch monday, or if you want it sooner feel free to do it

Comment 10

[Tarek Ziadé (:tarek)]

In the long run, I think the cleanest thing will be to trigger a 'flush' call when we reach n items in the child, and that can be checked everytime the counter is incremented. That will get rid of the counter altogether. I'll add a follow-up bug and work on this when I have more time.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1548540, reduce how often performance counters are sent to the parent process, r=tarek

Comment 12

[Tarek Ziadé (:tarek)]

Florian, I r+ the patch considering that 5s should not impact about:performance much since webext children are sending data every second right now and the UI is refreshing every 2 seconds (but we can tweak that value if needed -- before the beta release). When the UI is displayed, since they don't send the data at the same time, JS API dispatches are spreaded over a few seconds anyways and the trend is what really matters.

In the next version maybe we should add an explicit call to the children via the parent promise to have a better accuracy.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Anything which would get rid of the timer (at least when about:performance isn't open) would be great.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6dd8d9bf3e30
reduce how often performance counters are sent to the parent process, r=tarek

Comment 15

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/6dd8d9bf3e30