Closed
Bug 1062379
Opened 10 years ago
Closed 10 years ago
Don't reduce Gecko thread priority for multicore devices
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: jchen, Assigned: jchen)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mfinkle
:
review+
bnicholson
:
feedback+
|
Details | Diff | Splinter Review |
It's not very meaningful to reduce Gecko thread priority when the device has multiple cores.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Attachment #8483567 -
Flags: review?(mark.finkle)
Comment 2•10 years ago
|
||
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?
Assignee | ||
Comment 3•10 years ago
|
||
(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•10 years ago
|
||
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.
Attachment #8483567 -
Flags: review?(mark.finkle)
Attachment #8483567 -
Flags: review+
Attachment #8483567 -
Flags: feedback?(bnicholson)
Comment 5•10 years ago
|
||
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).
Attachment #8483567 -
Flags: feedback?(bnicholson) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•