1062379 - Don't reduce Gecko thread priority for multicore devices

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

It's not very meaningful to reduce Gecko thread priority when the device has multiple cores.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Don't reduce Gecko thread priority for multicore devices (v1)

For multicore devices, the Gecko thread should be able to run on a separate core than the UI thread, so there's no reason to lower the Gecko thread priority in consideration for the UI thread. Patch also fixes some possible race conditions involving sGeckoThread in ThreadUtils.

Comment 2

[Brian Nicholson (:bnicholson)]

Just curious: does lowering the priority of the Gecko thread hurt anything? If the threads are run on different cores, it seems like the lower priority wouldn't affect anything since there's no contention. Also, is there any guarantee that the Gecko/UI threads will actually be on separate cores for multi-core devices?

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Brian Nicholson (:bnicholson) from comment #2)
> Just curious: does lowering the priority of the Gecko thread hurt anything?
> If the threads are run on different cores, it seems like the lower priority
> wouldn't affect anything since there's no contention. Also, is there any
> guarantee that the Gecko/UI threads will actually be on separate cores for
> multi-core devices?

I guess it depends on the situation. Gecko thread could still be competing with other threads on the device or competing with the background thread. There's no guarantee but the scheduler should be smart enough to figure it out.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8483567 [details] [diff] [review]
Don't reduce Gecko thread priority for multicore devices (v1)

I don't mind trying this out. Brian spent a lot of time thinking about this code, so I want to be sure he doesn't have some words of warning.

Sometimes mucking with priorities has a way of doing the opposite thing you want, so let's keep an eye on performance after it lands.

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8483567 [details] [diff] [review]
Don't reduce Gecko thread priority for multicore devices (v1)

Review of attachment 8483567 [details] [diff] [review]:
-----------------------------------------------------------------

Since this code was based primarily on trial and error, my only recommendation would be to benchmark startup before and after your patch. When we landed the priority hack, we noticed a significant improvement in startup times for single core devices, so it could be useful to do the same here to make sure nothing regresses (and likewise, to see if anything improves).

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1abb419a83e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b1abb419a83e