Closed Bug 1071156 Opened 5 years ago Closed 5 years ago
Raise the Compositor Thread Priority
We should be able to composite at 60 fps even if we raise the compositor thread priority. Currently, if we raise the compositor priority to -1 or -4 (which is what android uses), lots of things slow down. Find out why this happens.
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
We could extend the profiler to be able to profile all the threads in the process. This would probably uncover threads that are used by the driver and might help us get a picture of what is happening on non gecko threads (gl driver). Filed bug 1071165 which might would help if we decide to go with that approach.
Raising it to -4 on a flame seems to work quite well. We tested scrolling around and browsing the web with audio playing in the background. Audio from the music app gets a priority of -16, video decoding gets a -2, aac decoding gets a -2 thread priority. My test case of nytimes.com, which checkerboards / long delays with touch seems to work with a compositor at -4. The video on the page works and audio plays in the background while loading nytimes.com until the video auto plays.
-4 so we match Android's compositor priority.
Attachment #8493367 - Flags: review?(roc)
Many of the issues I initially reported were from my memory a couple of months ago. Those things don't seem to be occurring anymore.
Summary: Find why raising the compositor priority thread makes everything else really slow → Raise the Compositor Thread Priority
Attachment #8493367 - Flags: review?(roc) → review+
(In reply to Mason Chang [:mchang] from comment #4) > Many of the issues I initially reported were from my memory a couple of > months ago. Those things don't seem to be occurring anymore. I'd be careful in testing this on a Flame only. The Flame is a dual-core device but most of our other phones aren't. This change might cause CPU starvation for other threads on single-core phones which wouldn't be visible on a Flame.
On ICS and JB, compositor thread was blocked longer time for composition if it try to compose layers.
(In reply to Sotaro Ikeda [:sotaro] from comment #6) > On ICS and JB, compositor thread was blocked longer time for composition if > it try to compose layers. It is hw platform dependent thing.
css-3d is one of very complex layer things. It might be better do testing in such situations. If we want to raise the priority more, we might need a lot of testings to confirm if there is no regression. Compositor thread activity could block kernel drivers work that has timing limitations.
We need to evaluate a situation that uses a lot of intermediate surface in compositor.
Not worried about pre-KK builds, and not worried about single core devices. There is nothing in the pipeline that fits that description.
Sorry, I misunderstood that the patch want to raise the priority to real-time priority.
FWIW I don't think we can starve the driver work since the will eventually block on the driver flushing the work. But it's possible that we will prevent it from starting the work earlier giving a performance regression.
Start up time measurements for moz-visually-app-complete: moz-app-visually-complete Compositor - -4 priority Calendar - 1651.06 Camera - 1860 Clock - 1136.77 Contacts - 1127 Dialer - 734.39 Email - 456 FM - 828 Gallery - 1102 Music - 1515 Settings - 2860 SMS - 1156 Video - 1087 Master - 0 priority Calendar - 1639 Camera - 1875 Contacts - 1131 Dialer - 722.4 Email - 474.6 FM - 791 Gallery - 1152 Settings - 2896 So sometimes a roughly ~10-15ms start up regression, which is the range of noise. Also, animations feel smoother, so even if we spend more time in the compositor for smoother animations at the expense of some start up time, it's better for the user.
(In reply to Mason Chang [:mchang] from comment #13) > So sometimes a roughly ~10-15ms start up regression, which is the range of > noise. Note that if this isn't noise it will show up on datazilla consistently so keep an eye on that after having landed this. That being said I agree that a single frame of delay is an acceptable price to pay for extra smoothness in animations and transitions.
Comment on attachment 8493367 [details] [diff] [review] Raise Compositor Priority to -4 Review of attachment 8493367 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/app/b2g.js @@ -736,5 @@ > // > // If RT priority is disabled, then the compositor nice value is used. The > // code will default to ANDROID_PRIORITY_URGENT_DISPLAY which is -8. Per gfx > // request we are keeping the compositor at nice level 0 until we can complete > // the investigation in bug 982972. This comment should be updated before this is committed, or it's going to look rather confusing?
Carrying r+, updated comment around the priority.
Try looks good. I also tested BenWa's suggestion of spin locking the compositor thread for 80ms per composite, which is attached. This represents a bad scenario where the compositor hogs the CPU. We see that the apps still load and we can still scroll, although janky which is expected. Loading apps took a small hit, but nothing unusable, which is quite good considering this is a very base scenario.
I want to track this as 2.2 "feature", because lot of people need to know about it...
feature-b2g: --- → 2.2+
I'm applying the change to test on my Nexus S, but for now it seems to be not that bad.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.