New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 618424 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
inactive
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Background threads priorities are are not properly set on Android

Project Member Reported by agrieve@chromium.org, Jun 8 2016

Issue description

For threads with a nice level of 10 or higher, Android's normal thread-priority setting functions also set the linux cgroup that the thread belongs to:

https://android.googlesource.com/platform/art/+/master/runtime/thread_android.cc

However, our code in base goes through this path for ThreadPriority::REALTIME_AUDIO, but not for ThreadPriority::BACKGROUND.

https://cs.chromium.org/chromium/src/base/threading/platform_thread_android.cc?l=42
 

Comment 1 by aelias@chromium.org, Jun 10 2016

Cc: aelias@chromium.org reve...@chromium.org lizeb@chromium.org ericrk@chromium.org
The only real use of ThreadPriority::BACKGROUND is for the background raster thread, which is (or is intended to be?) used for prepaint raster.  We intend this to run on a little core (which it does already), but the cgroup restriction to 5% of CPU is probably too much, we do want the raster to run at a reasonable pace still.  So I'm not sure we actually want to fix this.
If we could do some testing on a big.LITTLE device that would be nice. If we can restrict pre-paint without causing a noticeable degrade in the user experience then that's probably the way to go as it will save power and give more cpu resources to javascript or whatever else might have use for it on the system.

Comment 3 by lizeb@chromium.org, Jun 10 2016

There is a (hacky) way to restrict a thread to run on the little cores, with
sched_setaffinity().

This requires you to know which SoC you're running on, since the big / little ordering is not easily discoverable AFAIK. But it's doable, and it works (and covering a handful of popular SoCs is probably enough).

Owner: aelias@chromium.org
How do you know it runs only on a little core? I thought that's what the cgroup() enforced (which is what we're not doing). 

If we don't want to participate in the cgroup restrictions of nice=10, then we should probably just change it to use nice=9. 

Seeing that this applies only to raster thread, going to assign this aelias@.

Comment 5 by lizeb@chromium.org, Jun 10 2016

sched_setaffinity() takes a mask of CPUs (i.e. cores here) that a given thread is allowed to run on. If only the bits corresponding to the little cores are set, then the thread will only run on these cores. 

Comment 6 by aelias@chromium.org, Jun 10 2016

> How do you know it runs only on a little core?

I know because when the *main* raster thread was priority 10, it caused a serious checkerboarding performance issue on Nexus 5X (http://crbug.com/503720 ).

Comment 7 by aelias@chromium.org, Jun 10 2016

Status: WontFix (was: Assigned)
I think WontFixing this is fine because we like the current behavior, and I don't see a reason to go out of our way to tinker with sched_setaffinity() to get the same behavior with a different mechanism.  I think changes in this area should come out of "performance profiling indicates this should be changed".

Sign in to add a comment