Fix thread priorities for unstarted threads.

Calls to Thread.setPriority for unstarted threads now behave
similar to dalvik. Note that there's still some inconsistent
behaviour carried over from dalvik.

- high priority threads from bg_non_interactive processes are
  not always moved to the SP_FOREGROUND cgroup.
- we do not attempt to adjust the cgroup of a native thread
  that's attaching.

Note that on android, the system_server will change the
cgroups for all running threads in a process when it moves
into the foreground and background. It's by design that
threads in a background process can request to be moved
to the foreground by setting a higher priority.

bug: 17893086

(cherry picked from commit 1bd326a5e2aaff06a5bcae9cb2c42a4e8de31401)

Change-Id: Iad362f7c5c8697c349f2b6d7fcba69a4e141883e

diff --git a/runtime/thread.cc b/runtime/thread.cc
index 7d24562..2c44f27 100644 (file)
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -170,6 +170,9 @@ void* Thread::CreateCallback(void* arg) {
     self->GetJniEnv()->DeleteGlobalRef(self->tlsPtr_.jpeer);
     self->tlsPtr_.jpeer = nullptr;
     self->SetThreadName(self->GetThreadName(soa)->ToModifiedUtf8().c_str());
+
+    mirror::ArtField* priorityField = soa.DecodeField(WellKnownClasses::java_lang_Thread_priority);
+    self->SetNativePriority(priorityField->GetInt(self->tlsPtr_.opeer));
     Dbg::PostThreadStart(self);
 
     // Invoke the 'run' method of our java.lang.Thread.

diff --git a/runtime/thread_android.cc b/runtime/thread_android.cc
index 73a9e54..d5db983 100644 (file)
--- a/runtime/thread_android.cc
+++ b/runtime/thread_android.cc
@@ -55,6 +55,13 @@ void Thread::SetNativePriority(int newPriority) {
   int newNice = kNiceValues[newPriority-1];
   pid_t tid = GetTid();
 
+  // TODO: b/18249098 The code below is broken. It uses getpriority() as a proxy for whether a
+  // thread is already in the SP_FOREGROUND cgroup. This is not necessarily true for background
+  // processes, where all threads are in the SP_BACKGROUND cgroup. This means that callers will
+  // have to call setPriority twice to do what they want :
+  //
+  //     Thread.setPriority(Thread.MIN_PRIORITY);  // no-op wrt to cgroups
+  //     Thread.setPriority(Thread.MAX_PRIORITY);  // will actually change cgroups.
   if (newNice >= ANDROID_PRIORITY_BACKGROUND) {
     set_sched_policy(tid, SP_BACKGROUND);
   } else if (getpriority(PRIO_PROCESS, tid) >= ANDROID_PRIORITY_BACKGROUND) {

diff --git a/test/051-thread/expected.txt b/test/051-thread/expected.txt
index 943d1df..54e34af 100644 (file)
--- a/test/051-thread/expected.txt
+++ b/test/051-thread/expected.txt
@@ -9,4 +9,6 @@ testSleepZero finished
 testSetName starting
 testSetName running
 testSetName finished
+testThreadPriorities starting
+testThreadPriorities finished
 thread test done

diff --git a/test/051-thread/src/Main.java b/test/051-thread/src/Main.java
index 390685d..b81273e 100644 (file)
--- a/test/051-thread/src/Main.java
+++ b/test/051-thread/src/Main.java
@@ -20,12 +20,17 @@ import java.util.ArrayList;
  * Test some basic thread stuff.
  */
 public class Main {
+    static {
+        System.loadLibrary("arttest");
+    }
+
     public static void main(String[] args) throws Exception {
         System.out.println("thread test starting");
         testThreadCapacity();
         testThreadDaemons();
         testSleepZero();
         testSetName();
+        testThreadPriorities();
         System.out.println("thread test done");
     }
 
@@ -133,4 +138,53 @@ public class Main {
         }
         System.out.print("testSetName finished\n");
     }
+
+    private static void testThreadPriorities() throws Exception {
+        System.out.print("testThreadPriorities starting\n");
+
+        PriorityStoringThread t1 = new PriorityStoringThread(false);
+        t1.setPriority(Thread.MAX_PRIORITY);
+        t1.start();
+        t1.join();
+        if (supportsThreadPriorities() && (t1.getNativePriority() != Thread.MAX_PRIORITY)) {
+            System.out.print("thread priority for t1 was " + t1.getNativePriority() +
+                " [expected Thread.MAX_PRIORITY]\n");
+        }
+
+        PriorityStoringThread t2 = new PriorityStoringThread(true);
+        t2.start();
+        t2.join();
+        if (supportsThreadPriorities() && (t2.getNativePriority() != Thread.MAX_PRIORITY)) {
+            System.out.print("thread priority for t2 was " + t2.getNativePriority() +
+                " [expected Thread.MAX_PRIORITY]\n");
+        }
+
+        System.out.print("testThreadPriorities finished\n");
+    }
+
+    private static native int getNativePriority();
+    private static native boolean supportsThreadPriorities();
+
+    static class PriorityStoringThread extends Thread {
+        private final boolean setPriority;
+        private volatile int nativePriority;
+
+        public PriorityStoringThread(boolean setPriority) {
+            this.setPriority = setPriority;
+            this.nativePriority = -1;
+        }
+
+        @Override
+        public void run() {
+            if (setPriority) {
+                setPriority(Thread.MAX_PRIORITY);
+            }
+
+            nativePriority = Main.getNativePriority();
+        }
+
+        public int getNativePriority() {
+            return nativePriority;
+        }
+    }
 }

diff --git a/test/051-thread/thread_test.cc b/test/051-thread/thread_test.cc
new file mode 100644 (file)
index 0000000..2f5fffc
--- /dev/null
+++ b/test/051-thread/thread_test.cc
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jni.h"
+#include "thread-inl.h"
+
+namespace art {
+
+extern "C" JNIEXPORT jint JNICALL Java_Main_getNativePriority(JNIEnv* env, jclass) {
+  return ThreadForEnv(env)->GetNativePriority();
+}
+
+extern "C" JNIEXPORT jboolean JNICALL Java_Main_supportsThreadPriorities(JNIEnv* env, jclass) {
+#if defined(HAVE_ANDROID_OS)
+  return JNI_TRUE;
+#else
+  return JNI_FALSE;
+#endif
+}
+
+}  // namespace art

diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index 55de1f3..c9c0475 100644 (file)
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -24,6 +24,7 @@ LIBARTTEST_COMMON_SRC_FILES := \
   004-ReferenceMap/stack_walk_refmap_jni.cc \
   004-StackWalk/stack_walk_jni.cc \
   004-UnsafeTest/unsafe_test.cc \
+  051-thread/thread_test.cc \
   116-nodex2oat/nodex2oat.cc \
   117-nopatchoat/nopatchoat.cc \
   118-noimage-dex2oat/noimage-dex2oat.cc