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)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 35

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(1 file)

It's not very meaningful to reduce Gecko thread priority when the device has multiple cores.
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)
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?
(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 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 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+
Blocks: 807322
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: