content: Improve thread priority for raster threads.

base/threading/platform_thread_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/threading/platform_thread.h"
 
 #include <errno.h>
 #include <stddef.h>
 #include <sys/prctl.h>
 #include <sys/resource.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include "base/android/jni_android.h"
 #include "base/android/thread_utils.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/threading/platform_thread_internal_posix.h"
 #include "base/threading/thread_id_name_manager.h"
 #include "base/tracked_objects.h"
 #include "jni/ThreadUtils_jni.h"
 
 namespace base {
 
 namespace internal {
 
-// - BACKGROUND is 9 due to it being the nicest value we can use that's still
-// above an Android system threshold that enables heavy throttling starting at
-// 10; we want to be lower-priority than Chrome's other threads without
-// incurring this behavior.
+// - BACKGROUND corresponds to Android's PRIORITY_BACKGROUND = 10 value and can
+// result in heavy throttling.
 // - DISPLAY is -6 due to being midway between Android's DISPLAY (-4) and
 // URGENT_DISPLAY (-8).

[reveman, 2016/02/12 05:56:44]

Seems like a bit of an abuse to take URGENT_DISPLAY that is meant for ui
services on Android into account here. -4 not enough?

[aelias_OOO_until_Jul13, 2016/02/23 23:01:40]

Yeah, it seems a bit weird.  This was introduced in
https://chromiumcodereview.appspot.com/12741012 but there's no explanation for
the choice in the patch or code review discussion.  If you want to change it to
-4, fine by me.

[reveman, 2016/02/24 17:08:53]

Created a separate patch for this: https://codereview.chromium.org/1730053003

-// - REALTIME_AUDIO corresponds to Android's THREAD_PRIORITY_AUDIO = -16 value.
+// - REALTIME_AUDIO corresponds to Android's PRIORITY_AUDIO = -16 value.
 const ThreadPriorityToNiceValuePair kThreadPriorityToNiceValueMap[4] = {
-    {ThreadPriority::BACKGROUND, 9},
+    {ThreadPriority::BACKGROUND, 10},

[reveman, 2016/02/12 05:56:44]

Note that this will also make the renderer main thread use this priority when
smoothness is preferred.

[aelias_OOO_until_Jul13, 2016/02/23 23:01:40]

Yeah, that's concerning.  I don't think we want to force the main thread onto a
little core, that seems too aggressive.  Since you're significantly changing our
thread policy anyway, could you simply delete the main-thread-priority-change
effect of smoothness preferred?  It's one of those ugly complexities in our
scheduling policy, introduced back when we had a very different rendering
architecture (I believe it was in the days when raster was on main thread, so
the effect was primarily to throttle raster) -- I question its benefit so I
think it's time to try cleaning it out.

[reveman, 2016/02/24 17:08:54]

Removed it as part of this patch.

     {ThreadPriority::NORMAL, 0},
     {ThreadPriority::DISPLAY, -6},
     {ThreadPriority::REALTIME_AUDIO, -16},
 };
 
 bool SetCurrentThreadPriorityForPlatform(ThreadPriority priority) {
   // On Android, we set the Audio priority through JNI as Audio priority
   // will also allow the process to run while it is backgrounded.
   if (priority == ThreadPriority::REALTIME_AUDIO) {
     JNIEnv* env = base::android::AttachCurrentThread();
@@ skipping 50 matching lines @@
   // 1Mb is not enough for some tests (see http://crbug.com/263749 for example).
   return 2 * (1 << 20);  // 2Mb
 #endif
 }
 
 bool RegisterThreadUtils(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace base