Increase priority of raster threads on Android.

Increase priority of raster threads on Android.

On Android in particular, priority 10 is a threshold for qualitative
policy changes by the system that can throttle a thread heavily.  Raster
threads are really mid-priority threads that shouldn't interfere with
the threads responsible for issuing frames, but on the other hand
shouldn't be delayed for very long (or the user will see checkerboard on
the screen).

This patch changes the BACKGROUND priority to 9 on Android.  This also makes
compositor thread use that priority when smoothness does not take priority, as
well as the async upload thread, both of which seem appropriate as well.
(There are no other users of BACKGROUND priority outside the graphics stack.)

BUG=503720
Committed: https://crrev.com/e8687a393c617f54fe1e1bf8a79b204a69a10dfb
Cr-Commit-Position: refs/heads/master@{#337532}

[aelias_OOO_until_Jul13, 2015-06-25 22:16:17]

aelias@chromium.org changed reviewers:
+ danakj@chromium.org, sievers@chromium.org

[aelias_OOO_until_Jul13, 2015-06-25 22:16:46]

danakj@ for base/ review (usual threading reviewer rvargas@ is on leave)
sievers@ for content/ review

[no sievers, 2015-06-25 22:52:16]

lgtm

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
File base/threading/platform_thread_freebsd.cc (right):

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
base/threading/platform_thread_freebsd.cc:32: const
ThreadPriorityToNiceValuePair kThreadPriorityToNiceValueMap[4] = {
[4] -> [5]

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
File base/threading/platform_thread_win.cc (right):

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
base/threading/platform_thread_win.cc:243: case ThreadPriority::UI_BACKGROUND:
Now I'm wondering if BELOW_NORMAL is actually a better name than UI_BACKGROUND.
Don't feel strongly though...

[aelias_OOO_until_Jul13, 2015-06-25 23:49:13]

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
File base/threading/platform_thread_freebsd.cc (right):

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
base/threading/platform_thread_freebsd.cc:32: const
ThreadPriorityToNiceValuePair kThreadPriorityToNiceValueMap[4] = {
On 2015/06/25 at 22:52:15, sievers wrote:
> [4] -> [5]

Done.

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
File base/threading/platform_thread_win.cc (right):

https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
base/threading/platform_thread_win.cc:243: case ThreadPriority::UI_BACKGROUND:
On 2015/06/25 at 22:52:15, sievers wrote:
> Now I'm wondering if BELOW_NORMAL is actually a better name than
UI_BACKGROUND. Don't feel strongly though...

I chose UI_BACKGROUND because the other priority names ("REALTIME_AUDIO") refer
to use cases.  Maybe "UI_BACKGROUND" doesn't communicate the use case that well
though... what do you think about "BACKGROUND_GRAPHICS"?

[no sievers, 2015-06-26 01:02:29]

On 2015/06/25 23:49:13, aelias wrote:
>
https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
> File base/threading/platform_thread_freebsd.cc (right):
> 
>
https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
> base/threading/platform_thread_freebsd.cc:32: const
> ThreadPriorityToNiceValuePair kThreadPriorityToNiceValueMap[4] = {
> On 2015/06/25 at 22:52:15, sievers wrote:
> > [4] -> [5]
> 
> Done.
> 
>
https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
> File base/threading/platform_thread_win.cc (right):
> 
>
https://codereview.chromium.org/1214503002/diff/1/base/threading/platform_thr...
> base/threading/platform_thread_win.cc:243: case ThreadPriority::UI_BACKGROUND:
> On 2015/06/25 at 22:52:15, sievers wrote:
> > Now I'm wondering if BELOW_NORMAL is actually a better name than
> UI_BACKGROUND. Don't feel strongly though...
> 
> I chose UI_BACKGROUND because the other priority names ("REALTIME_AUDIO")
refer
> to use cases.  Maybe "UI_BACKGROUND" doesn't communicate the use case that
well
> though... what do you think about "BACKGROUND_GRAPHICS"?

sgtm

[danakj, 2015-07-06 22:49:49]

> This patch introduces a new thread priority level mapped to 9 on POSIX and
BELOW_NORMAL on Windows, and uses it for the raster threads.  I also made the
compositor thread use that priority when smoothness does not take priority,
which seems like another appropriate use of it.

I'm feeling dubious about this change. Here are my thoughts:

- When I put my video encoding to Below Normal on windows, the encode takes a
lot longer, even though the system is somewhat idle. It feels to me like the
process is getting descheduled a lot "just in case" or something. Have I been
making that up?

- Lowering the priority of the compositor thread means we'll schedule java
script over the production of the last frame that blink gave to cc. That sounds
like something that will increase blink's latency to screen.

- Prioritizing the compositor thread over raster means we'll always be extra
sure to get a (checkerboarded) frame up on the screen. But maybe we could have
actually got rastered content up instead.

What kind of data do you have to motivate this change?

[danakj, 2015-07-06 23:10:01]

OK sorry I read this wrong.. we're making them increased priority.. like the
title.. wee ! Anyhow I think this could just change the BACKGROUND nice value,
and add a comment explaining the magic number please.

[aelias_OOO_until_Jul13, 2015-07-06 23:21:37]

On 2015/07/06 at 23:10:01, danakj wrote:
> OK sorry I read this wrong.. we're making them increased priority.. like the
title.. wee ! Anyhow I think this could just change the BACKGROUND nice value,
and add a comment explaining the magic number please.

Done, PTAL.

[danakj, 2015-07-06 23:24:22]

LGTM

https://codereview.chromium.org/1214503002/diff/60001/base/threading/platform...
File base/threading/platform_thread_android.cc (right):

https://codereview.chromium.org/1214503002/diff/60001/base/threading/platform...
base/threading/platform_thread_android.cc:26: // - BACKGROUND is 9 due to it
being the nicest value we can use that's still
Thanks this comment is awesome.

[aelias_OOO_until_Jul13, 2015-07-06 23:26:26]

The CQ bit was checked by aelias@chromium.org

[aelias_OOO_until_Jul13, 2015-07-06 23:26:26]

The patchset sent to the CQ was uploaded after l-g-t-m from sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1214503002/#ps60001
(title: "Just change existing value to 9")

[commit-bot: I haz the power, 2015-07-06 23:27:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214503002/60001

[commit-bot: I haz the power, 2015-07-07 00:34:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-07-07 00:35:41]

Patchset 4 (id:??) landed as
https://crrev.com/e8687a393c617f54fe1e1bf8a79b204a69a10dfb
Cr-Commit-Position: refs/heads/master@{#337532}