Closed Bug 1553131 Opened 3 years ago Closed 2 years ago

Run raptor-youtube-playback jobs with profiler enabled in CI

Categories

(Testing :: Raptor, defect, P2)

defect

Tracking

(firefox74 fixed)

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: whimboo, Assigned: marauder)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

Priority: -- → P2
Summary: Run raptor-youtube-playback jobs with profiler by default → Run raptor-youtube-playback jobs with profiler enabled in CI
Assignee: nobody → marian.raiciof
Status: NEW → ASSIGNED
Pushed by mraiciof@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7fc8ef7f16b
Run raptor-youtube-playback jobs with profiler enabled in CI r=perftest-reviewers,sparky

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

Flags: needinfo?(sheriffs)
Flags: needinfo?(marian.raiciof)

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?

Flags: needinfo?(sheriffs) → needinfo?(padenot)

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

Flags: needinfo?(alwu)

(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?

Flags: needinfo?(padenot) → needinfo?(jyavenard)

(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

Flags: needinfo?(jyavenard)

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.

Flags: needinfo?(alwu)

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

Flags: needinfo?(marian.raiciof)

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!

Flags: needinfo?(tarek)
Flags: needinfo?(gmierz2)

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

Flags: needinfo?(gmierz2)

sounds good. we can always revisit

Flags: needinfo?(tarek)

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!

Pushed by mraiciof@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4e0e11d0e767
Run raptor-youtube-playback jobs with profiler enabled in CI r=perftest-reviewers,sparky,tarek
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
You need to log in before you can comment on or make changes to this bug.