Set chrome thread name in JVM.

base/android/jni_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/android/jni_android.h"
 
 #include <map>
 
 #include "base/android/build_info.h"
 #include "base/android/jni_string.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/threading/platform_thread.h"
+#include "base/threading/thread_local.h"
 
 namespace {
 using base::android::GetClass;
 using base::android::MethodID;
 using base::android::ScopedJavaLocalRef;
 
 JavaVM* g_jvm = NULL;
 // Leak the global app context, as it is used from a non-joinable worker thread
 // that may still be running at shutdown. There is no harm in doing this.
 base::LazyInstance<base::android::ScopedJavaGlobalRef<jobject> >::Leaky
     g_application_context = LAZY_INSTANCE_INITIALIZER;
 
+// Whether or not AttachCurrentThread() has been called before.
+base::LazyInstance<base::ThreadLocalBoolean>::Leaky already_attached =
+    LAZY_INSTANCE_INITIALIZER;
+
 std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) {
   ScopedJavaLocalRef<jclass> throwable_clazz =
       GetClass(env, "java/lang/Throwable");
   jmethodID throwable_printstacktrace =
       MethodID::Get<MethodID::TYPE_INSTANCE>(
           env, throwable_clazz.obj(), "printStackTrace",
           "(Ljava/io/PrintStream;)V");
 
   // Create an instance of ByteArrayOutputStream.
   ScopedJavaLocalRef<jclass> bytearray_output_stream_clazz =
@@ skipping 34 matching lines @@
 }
 
 }  // namespace
 
 namespace base {
 namespace android {
 
 JNIEnv* AttachCurrentThread() {
   DCHECK(g_jvm);
   JNIEnv* env = NULL;
-  jint ret = g_jvm->AttachCurrentThread(&env, NULL);
+  jint ret = JNI_OK;
+  // Passes thread name in args for first call.
+  if (!already_attached.Get().Get()) {

[bulach, 2014/06/17 17:26:55]

hmm... AttachCurrentThread is a somewhat hot function, called on tons of places.
how about moving this somewhere higher up, say something like:
PlatformThread::SetName?

https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/pla...

would that be sufficient?

[byungchul, 2014/06/17 17:51:05]

Do you mean call AttachCurrentThread() in PlatformThread::SetName()? It creates
unnecessary thread objects in jvm. And, DetachFromVM() will undo it though it
may be rarely called.
To avoid lock in lazy instance, thread local boolean could be initailized in
InitVM() below.
If you concern about thread local boolean which calls pthread_get_specific(),
AttachCurrentThread() also calls pthread_get_specific() in
https://cs.corp.google.com/#android/dalvik/vm/Thread.cpp&l=1009 to fetch jvm
structure.

[bulach, 2014/06/17 20:45:33]

tough choice :)
how many named threads which are never Attached to the VM there is?
vs...how many times is AttachCurrentThread() be called?
I suppose the overhead for the unnecessary objects may be smaller than the extra
calls here, but I don't have the numbers...
any chance we could investigate that?
yeah, that should only be called on thread destruction, so shouldn't be a
problem.
sure, there's all sorts of hidden costs in crossing the boundaries :)
but it'd be really nice to minimize it, specially in hot functions such as
this...
if the SetName turns out being less frequent, it'd better to call at that level.

[byungchul, 2014/06/17 21:11:11]

I am not working on android chrome, but a different project utilizing it. Here
is a snapshot of threads after masking some thread names. Thread-??? is the name
assigned by jvm. So, 14 of 52 threads will have benefit for this change while we
may create jvm thread object up to 38 more.

XXXXXXX
Heap
Heap
Heap
Signal
JDWP
ReferenceQueueD
FinalizerDaemon
FinalizerWatchd
HeapTrimmerDaem
GCDaemon
Binder_1
Binder_2
AsyncTask
pool-1-thread-1
RefQueueWorker@
DnsConfigServic
inotify_reader
Thread-119
SettingsObserve
Chrome_DBThread
Thread-114
Chrome_FileUser
Thread-115
Chrome_CacheThr
Thread-103
IndexedDB
XXXXXXX
XXXXXXX
XXXXXXX
Thread-105
Thread-107
File
Thread-111
Thread-113
XXXXXXX
pool-2-thread-1
pool-3-thread-1
NsdManager
Thread-107
Thread-107
Thread-107
Thread-107
Thread-107
Thread-107
ConfigFileIOWor
SimpleCacheWork
RenderThread
Chrome_DevTools
AsyncTransferTh
JavaBridge
Binder_3

+    already_attached.Get().Set(true);
+    JavaVMAttachArgs args;
+    args.version = JNI_VERSION_1_2;
+    args.name = PlatformThread::GetName();
+    args.group = NULL;
+    ret = g_jvm->AttachCurrentThread(&env, &args);
+  } else {
+    ret = g_jvm->AttachCurrentThread(&env, NULL);
+  }
   DCHECK_EQ(JNI_OK, ret);
   return env;
 }
 
 void DetachFromVM() {
   // Ignore the return value, if the thread is not attached, DetachCurrentThread
   // will fail. But it is ok as the native thread may never be attached.
-  if (g_jvm)
+  if (g_jvm) {
     g_jvm->DetachCurrentThread();
+    already_attached.Get().Set(false);
+  }
 }
 
 void InitVM(JavaVM* vm) {
   DCHECK(!g_jvm);
   g_jvm = vm;
 }
 
 bool IsVMInitialized() {
   return g_jvm != NULL;
 }
@@ skipping 100 matching lines @@
   // RVO should avoid any extra copies of the exception string.
   base::android::BuildInfo::GetInstance()->set_java_exception_info(
       GetJavaExceptionInfo(env, java_throwable));
 
   // Now, feel good about it and die.
   CHECK(false);
 }
 
 }  // namespace android
 }  // namespace base