1366358 - [meta] Be better at relative scheduling of our thread priorities

Status: NEW

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[(no longer active)]

We currently can run into scenarios where our compositor thread gets starved when a background content process main thread runs because we don't correctly prioritize our threads respectively.  We need to get better at this.

mconley noted this in a profile earlier today.  Do you mind posting a link to it for reference please?

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Here we go: https://perf-html.io/public/3e4be8ee6a3218e5d5460331df5a26a100bbe8e6/calltree/?invertCallstack&range=11.6500_14.8054~11.9497_12.0492&thread=5

Comment 2

[Andrew Overholt [:overholt]]

meta = P3

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Naveed, if you are doing performance triage again we should consider prioritizing this one.  Its a known perf issue for firefox running on low end hardware.  We need to avoid starving things like the compositor thread, etc.

Comment 4

[Andrew Overholt [:overholt]]

Is there anything to be done in this bug or is the work in the dependencies?

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

I think its in the bug dependencies.  It would be nice to have someone owning and looking at this collection of bugs, though.

Comment 6

[Gabriele Svelto [:gsvelto]]

Raising the priority of the compositor thread gave us significant benefits on Fx0S (see bug 980027). Doing it on desktop is going to be tricky though, there's not much you can do to thread priorities unless your root/admin.

Comment 7

[(no longer active)]

(In reply to Gabriele Svelto [:gsvelto] from comment #6)
> Raising the priority of the compositor thread gave us significant benefits
> on Fx0S (see bug 980027). Doing it on desktop is going to be tricky though,
> there's not much you can do to thread priorities unless your root/admin.

Oh, you require admin privileges in order to change thread priorities?  :-(  I didn't know that.

I looked up the code Chromium uses for this.  I don't know if their code manages to work without admin privileges or not, but I thought gathering this info here might help you tell quickly by looking at their code.  Here is what they do on Linux:

https://cs.chromium.org/chromium/src/base/threading/platform_thread_linux.cc?l=163&rcl=d271f4b26d827195328939f220b084c49567af1c
Here are the nice values they use:
https://cs.chromium.org/chromium/src/base/threading/platform_thread_linux.cc?l=93&rcl=80f51807028d3bc23f16f2572e565a23f823469b

On Mac...  hmm, it's complicated.  :D  We can probably copy what they have (I didn't spend time to try to grok it all):
https://cs.chromium.org/chromium/src/base/threading/platform_thread_mac.mm?l=156&rcl=29cc37fa205ee4a8b0c0ff1042673d6b26857508

Windows:
https://cs.chromium.org/chromium/src/base/threading/platform_thread_win.cc?l=289&rcl=3a73f86ed1d1f36b0ff7b4fa7c36bb1c2a8f6d8e
Priority values are listed in the same function.

On Android, they use the same code as Linux, but for their realtime audio thread they use the JNI API to set the thread priority <https://cs.chromium.org/chromium/src/base/threading/platform_thread_android.cc?l=37&rcl=3a73f86ed1d1f36b0ff7b4fa7c36bb1c2a8f6d8e> and they also use different nice values: <https://cs.chromium.org/chromium/src/base/threading/platform_thread_android.cc?l=25&rcl=3a73f86ed1d1f36b0ff7b4fa7c36bb1c2a8f6d8e>.

Comment 8

[Gabriele Svelto [:gsvelto]]

(In reply to :Ehsan Akhgari from comment #7)
> Oh, you require admin privileges in order to change thread priorities?  :-( 
> I didn't know that.

With regular round-robin scheduling (SCHED_OTHER) on Linux you can always lower a thread priority but you can never raise it again (unless you're root or have the CAP_SYS_NICE capability). We already lower the priority of some background threads, look for PR_PRIORITY_LOW instances in the codebase.

I haven't tested on Windows but there's a dedicated group permission to allow users to raise thread priorities so I doubt a regular application can do that.

> On Android, they use the same code as Linux, but for their realtime audio
> thread they use the JNI API to set the thread priority
> <https://cs.chromium.org/chromium/src/base/threading/platform_thread_android.
> cc?l=37&rcl=3a73f86ed1d1f36b0ff7b4fa7c36bb1c2a8f6d8e> and they also use
> different nice values:
> <https://cs.chromium.org/chromium/src/base/threading/platform_thread_android.
> cc?l=25&rcl=3a73f86ed1d1f36b0ff7b4fa7c36bb1c2a8f6d8e>.

Android is special in that regard because it provides special priority classes that are managed via cgroups internally. This allows for applications to request special priorities without the risk of starving the system due to a misbehaving one.

Comment 9

[(no longer active)]

(In reply to Gabriele Svelto [:gsvelto] from comment #8)
> (In reply to :Ehsan Akhgari from comment #7)
> > Oh, you require admin privileges in order to change thread priorities?  :-( 
> > I didn't know that.
> 
> With regular round-robin scheduling (SCHED_OTHER) on Linux you can always
> lower a thread priority but you can never raise it again (unless you're root
> or have the CAP_SYS_NICE capability). We already lower the priority of some
> background threads, look for PR_PRIORITY_LOW instances in the codebase.

Hmm, I tested on Linux, and when running Chrome as non-root on Ubuntu, it lowers the priority of some threads to a nice value of 10 (corresponding to ThreadPriority::BACKGROUND) but nothing gets a high priority (their audio and compositor threads stay at the same priority).  So it seems like they haven't tested that code, or it's intended to only work when the user has enough privileges or something.

> I haven't tested on Windows but there's a dedicated group permission to
> allow users to raise thread priorities so I doubt a regular application can
> do that.

jrmuizel and I tested on Windows.  There Chrome seems to successfully be able to raise the priority of things like their GPU process.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

> Hmm, I tested on Linux, and when running Chrome as non-root on Ubuntu, it
> lowers the priority of some threads to a nice value of 10 (corresponding to
> ThreadPriority::BACKGROUND) but nothing gets a high priority (their audio
> and compositor threads stay at the same priority).  So it seems like they
> haven't tested that code, or it's intended to only work when the user has
> enough privileges or something.

It might work on ChromeOS, perhaps.  And their code for linux does seem to poke at cgroups, so it may work there.

Note that our webrtc code uses media/webrtc/trunk/webrtc/base/platform_thread.cc; for linux/mac/android it uses pthread_setschedparam() with SCHED_RR -- which basically fails (on Linux at least).  Interestingly, nothing in the webrtc code seems to use kNormalPriority or kLowPriority (except for a unit test).  On windows, it uses SetThreadPriority() with THREAD_PRIORITY_HIGHEST and THREAD_PRIORITY_TIME_CRITICAL.  Since (I believe) we're NORMAL_PRIORITY_CLASS, that gives a TIME_CRITICAL thread a base priority of 15, which is higher than any other priority for threads other than those in a REALTIME_PRIORITY_CLASS process.  So on windows, the webrtc threads (that want higher priority - there are about 8 or so of them that use HIGHEST and TIME_CRITICAL) will get high priority.

> > I haven't tested on Windows but there's a dedicated group permission to
> > allow users to raise thread priorities so I doubt a regular application can
> > do that.
> 
> jrmuizel and I tested on Windows.  There Chrome seems to successfully be
> able to raise the priority of things like their GPU process.

Right - and there are thread priorities within the classes as defined -- see https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100(v=vs.85).aspx and https://msdn.microsoft.com/en-us/library/windows/desktop/ms686277(v=vs.85).aspx

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Poking some media folk for awareness

Comment 12

[Gabriele Svelto [:gsvelto]]

(In reply to Randell Jesup [:jesup] from comment #10)
> It might work on ChromeOS, perhaps.  And their code for linux does seem to
> poke at cgroups, so it may work there.

Yeah, since they control the whole stack it's likely that Chrome has all the necessary privileges there. After all we had the same on FxOS and it worked - including realtime scheduling. If this works on vanilla Windows though it would be a huge boon on our biggest platform. Putting the compositor thread at a higher priority caused some minor regressions on FxOS but in general it provided a much, much smoother user experience.