Currently the YouTube playback performance tests do not run with the gecko profiler enabled in CI. If we are going to do that on m-c and integration branches we would drastically have to reduce the interval and maybe bump the number of samples. Otherwise data as sampled at the beginning of the test is lost due to its rotated recording.

Given that profiles might still be large, it would be good to wait for bug 1550702 first.

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

As explained in the Phabricator review this patch shouldn't have been landed. Please back out the above changeset. Thanks.

Backout: https://hg.mozilla.org/integration/autoland/rev/d326f257b043eeaaa6da29ef2015c996c5ad58ca

When i check the profile from the following job I can see that MediaPlayback threads are accumulating, and never seem to be destroyed. Means that we have more than 60 or so threads present for the WebContent process.

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

Paul, is that expected? Or should we be more aggressive in destroying no-longer needed threads?

Alastor do you have any idea if Henrik's observation in comment #6 is to be expected or not?

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

Paul, is that expected? Or should we be more aggressive in destroying no-longer needed threads?

jya, do we do anything to shut down the thread pool backing the task queue?

(In reply to Paul Adenot (:padenot) from comment #8)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

Paul, is that expected? Or should we be more aggressive in destroying no-longer needed threads?

jya, do we do anything to shut down the thread pool backing the task queue?

The media code use a SharedThreadPool, it gets automatically shutdown (and all threads it contains) as soon as it's no longer in use and now one is referencing it.

The SharedThreadPool will create as many threads as needed to run all tasks given to it, so long that the thread limit isn't reached (which is currently 4 by default).

There are a few SharedThreadPool in use (playback, msg, mdsm) etc. but it can't go to 60.

There is an issue however with how a nsThreadPool destroy a thread. It queue a task to the main thread. If the SharedThreadPool is used on the main thread and some code is blocking the main thread, the task to free the thread will never run and so the number of threads could look like it's creeping. I have opened that bug a few weeks ago (bug 1573072).

However, I'm not sure this is what is happening here.

In short, unless you're in the situation I described just before, there's no way the media SharedThreadPool could have 60 threads in it.
https://searchfox.org/mozilla-central/source/xpcom/threads/nsThreadPool.cpp#87

I think that it's time to move off this discussion to a different media bug. As such I filed bug 1576135. Thanks for the replies so far.

Henrik, i created an excel file with the values i tested so far for gecko_profile_interval and gecko_profile_entries.
https://docs.google.com/spreadsheets/d/1PnMRFmdurAKLQDz4W1OH6ZOv7Ud7R_vRwzOvs4UFTAo/edit#gid=0

https://phabricator.services.mozilla.com/D42516#1385770

Hey Tarek, Greg,

Can you take a look at the last comments from https://phabricator.services.mozilla.com/D42516
and decide the values that we should use for gecko_profile_entries ?

The gecko_profile_interval should be 1000, that is fine.

The last measurements are in the last comment:
https://phabricator.services.mozilla.com/D42516#1495055

Thanks!

:tarek what do you think? I'm thinking that 50,000,000 would be a happy middle from the last measurements that were provided.

sounds good. we can always revisit

I rebased and added the last changes.
I did another push to try, just to be sure:
https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=1cee01f45c565f4b2e6967c52129521c302f80e6

Thank you!

https://hg.mozilla.org/mozilla-central/rev/4e0e11d0e767