Android: lazy initialization for method id.

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/atomicops.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/threading/platform_thread.h"
 
 namespace {
 using base::android::GetClass;
-using base::android::GetMethodID;
+using base::android::MethodID;
 using base::android::ScopedJavaLocalRef;
 
 struct MethodIdentifier {
   const char* class_name;
   const char* method;
   const char* jni_signature;
 
   bool operator<(const MethodIdentifier& other) const {
     int r = strcmp(class_name, other.class_name);
     if (r < 0) {
@@ skipping 23 matching lines @@
 // 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;
 base::LazyInstance<MethodIDMap>::Leaky
     g_method_id_map = LAZY_INSTANCE_INITIALIZER;
 
 std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) {
   ScopedJavaLocalRef<jclass> throwable_clazz =
       GetClass(env, "java/lang/Throwable");
   jmethodID throwable_printstacktrace =
-      GetMethodID(env, throwable_clazz, "printStackTrace",
-                  "(Ljava/io/PrintStream;)V");
+      MethodID::Get<MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_YES>(
+          env, throwable_clazz.obj(), "printStackTrace",
+          "(Ljava/io/PrintStream;)V");
 
   // Create an instance of ByteArrayOutputStream.
   ScopedJavaLocalRef<jclass> bytearray_output_stream_clazz =
       GetClass(env, "java/io/ByteArrayOutputStream");
   jmethodID bytearray_output_stream_constructor =
-      GetMethodID(env, bytearray_output_stream_clazz, "<init>", "()V");
+      MethodID::Get<MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_YES>(
+          env, bytearray_output_stream_clazz.obj(), "<init>", "()V");
   jmethodID bytearray_output_stream_tostring =
-      GetMethodID(env, bytearray_output_stream_clazz, "toString",
-                  "()Ljava/lang/String;");
+      MethodID::Get<MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_YES>(
+          env, bytearray_output_stream_clazz.obj(), "toString",
+          "()Ljava/lang/String;");
   ScopedJavaLocalRef<jobject> bytearray_output_stream(env,
       env->NewObject(bytearray_output_stream_clazz.obj(),
                      bytearray_output_stream_constructor));
 
   // Create an instance of PrintStream.
   ScopedJavaLocalRef<jclass> printstream_clazz =
       GetClass(env, "java/io/PrintStream");
   jmethodID printstream_constructor =
-      GetMethodID(env, printstream_clazz, "<init>",
-                  "(Ljava/io/OutputStream;)V");
+      MethodID::Get<MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_YES>(
+          env, printstream_clazz.obj(), "<init>",
+          "(Ljava/io/OutputStream;)V");
   ScopedJavaLocalRef<jobject> printstream(env,
       env->NewObject(printstream_clazz.obj(), printstream_constructor,
                      bytearray_output_stream.obj()));
 
   // Call Throwable.printStackTrace(PrintStream)
   env->CallVoidMethod(java_throwable, throwable_printstacktrace,
       printstream.obj());
 
   // Call ByteArrayOutputStream.toString()
   ScopedJavaLocalRef<jstring> exception_string(
       env, static_cast<jstring>(
           env->CallObjectMethod(bytearray_output_stream.obj(),
                                 bytearray_output_stream_tostring)));
 
   return ConvertJavaStringToUTF8(exception_string);
 }
 
-enum MethodType {
-  METHODTYPE_STATIC,
-  METHODTYPE_NORMAL,
-};
-
-enum ExceptionCheck {
-  EXCEPTIONCHECK_YES,
-  EXCEPTIONCHECK_NO,
-};
-
-template<MethodType method_type, ExceptionCheck exception_check>
-jmethodID GetMethodIDInternal(JNIEnv* env,
-                              jclass clazz,
-                              const char* method_name,
-                              const char* jni_signature) {
-  jmethodID method_id = method_type == METHODTYPE_STATIC ?
-      env->GetStaticMethodID(clazz, method_name, jni_signature) :
-      env->GetMethodID(clazz, method_name, jni_signature);
-  if (exception_check == EXCEPTIONCHECK_YES) {
-    CHECK(!base::android::ClearException(env) && method_id) <<
-      "Failed to find " <<
-      (method_type == METHODTYPE_STATIC ? "static " : "") <<
-      "method " << method_name << " " << jni_signature;
-  } else if (base::android::HasException(env)) {
-    env->ExceptionClear();
-  }
-  return method_id;
-}
-
 }  // namespace
 
 namespace base {
 namespace android {
 
 JNIEnv* AttachCurrentThread() {
   if (!g_jvm)
     return NULL;
   JNIEnv* env = NULL;
   jint ret = g_jvm->AttachCurrentThread(&env, NULL);
@@ skipping 37 matching lines @@
   ScopedJavaLocalRef<jclass> clazz(env, env->FindClass(class_name));
   if (!clazz.obj()) {
     ClearException(env);
     return false;
   }
   bool error = ClearException(env);
   DCHECK(!error);
   return true;
 }
 
-jmethodID GetMethodID(JNIEnv* env,
-                      const JavaRef<jclass>& clazz,
-                      const char* method_name,
-                      const char* jni_signature) {
-  // clazz.env() can not be used as that may be from a different thread.
-  return GetMethodID(env, clazz.obj(), method_name, jni_signature);
+template<MethodID::MethodType method_type,
+         MethodID::ExceptionCheck exception_check>
+jmethodID MethodID::Get(JNIEnv* env,
+                        jclass clazz,
+                        const char* method_name,
+                        const char* jni_signature) {
+  jmethodID id = method_type == METHODTYPE_STATIC ?
+      env->GetStaticMethodID(clazz, method_name, jni_signature) :
+      env->GetMethodID(clazz, method_name, jni_signature);
+  if (exception_check == EXCEPTIONCHECK_YES) {
+    CHECK(base::android::ClearException(env) || id) <<
+        "Failed to find " <<
+        (method_type == METHODTYPE_STATIC ? "static " : "") <<
+        "method " << method_name << " " << jni_signature;
+  } else if (base::android::HasException(env)) {
+    env->ExceptionClear();
+  }
+  return id;
 }
 
-jmethodID GetMethodID(JNIEnv* env,
-                      jclass clazz,
-                      const char* method_name,
-                      const char* jni_signature) {
-  return GetMethodIDInternal<METHODTYPE_NORMAL, EXCEPTIONCHECK_YES>(
+template<MethodID::MethodType method_type,
+         MethodID::ExceptionCheck exception_check>
+jmethodID MethodID::LazyGet(JNIEnv* env,
+                            jclass clazz,
+                            const char* method_name,
+                            const char* jni_signature,
+                            base::subtle::AtomicWord* atomic_method_id) {
+  COMPILE_ASSERT(sizeof(subtle::AtomicWord) >= sizeof(jmethodID),
+                 AtomicWord_SmallerThan_jMethodID);
+  subtle::AtomicWord value = base::subtle::Acquire_Load(atomic_method_id);
+  if (value)
+    return reinterpret_cast<jmethodID>(value);
+  jmethodID id = MethodID::Get<method_type, exception_check>(
       env, clazz, method_name, jni_signature);
+  base::subtle::Release_Store(
+      atomic_method_id, reinterpret_cast<subtle::AtomicWord>(id));
+  return id;
 }
 
-jmethodID GetMethodIDOrNull(JNIEnv* env,
-                            jclass clazz,
-                            const char* method_name,
-                            const char* jni_signature) {
-  return GetMethodIDInternal<METHODTYPE_NORMAL, EXCEPTIONCHECK_NO>(
-      env, clazz, method_name, jni_signature);
-}
+// Various template instantiations.
+template jmethodID MethodID::Get<MethodID::METHODTYPE_STATIC,
+                                 MethodID::EXCEPTIONCHECK_YES>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature);
 
-jmethodID GetStaticMethodID(JNIEnv* env,
-                            const JavaRef<jclass>& clazz,
-                            const char* method_name,
-                            const char* jni_signature) {
-  return GetStaticMethodID(env, clazz.obj(), method_name,
-      jni_signature);
-}
+template jmethodID MethodID::Get<MethodID::METHODTYPE_NORMAL,
+                                 MethodID::EXCEPTIONCHECK_YES>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature);
 
-jmethodID GetStaticMethodID(JNIEnv* env,
-                            jclass clazz,
-                            const char* method_name,
-                            const char* jni_signature) {
-  return GetMethodIDInternal<METHODTYPE_STATIC, EXCEPTIONCHECK_YES>(
-      env, clazz, method_name, jni_signature);
-}
+template jmethodID MethodID::Get<MethodID::METHODTYPE_STATIC,
+                                 MethodID::EXCEPTIONCHECK_NO>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature);
 
-jmethodID GetStaticMethodIDOrNull(JNIEnv* env,
-                                  jclass clazz,
-                                  const char* method_name,
-                                  const char* jni_signature) {
-  return GetMethodIDInternal<METHODTYPE_STATIC, EXCEPTIONCHECK_NO>(
-      env, clazz, method_name, jni_signature);
-}
+template jmethodID MethodID::Get<MethodID::METHODTYPE_NORMAL,
+                                 MethodID::EXCEPTIONCHECK_NO>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature);

[joth, 2012/10/04 17:59:59]

fwiw the ::Get() instantiations are not strictly needed as LazyGet calls Get()
so they'll happen as side effect anyway. (Probably best to keep them all though
for clarity?)

[bulach, 2012/10/04 18:58:17]

I'd love to delete, but I think this makes it clearer and will later avoid heads
being scratched on why one and not the other? :)

 
-bool HasMethod(JNIEnv* env,
-               const JavaRef<jclass>& clazz,
-               const char* method_name,
-               const char* jni_signature) {
-  jmethodID method_id =
-      env->GetMethodID(clazz.obj(), method_name, jni_signature);
-  if (!method_id) {
-    ClearException(env);
-    return false;
-  }
-  bool error = ClearException(env);
-  DCHECK(!error);
-  return true;
-}
+template jmethodID MethodID::LazyGet<MethodID::METHODTYPE_STATIC,
+                                     MethodID::EXCEPTIONCHECK_YES>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature, base::subtle::AtomicWord* atomic_method_id);
+
+template jmethodID MethodID::LazyGet<MethodID::METHODTYPE_NORMAL,
+                                     MethodID::EXCEPTIONCHECK_YES>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature, base::subtle::AtomicWord* atomic_method_id);
+
+template jmethodID MethodID::LazyGet<MethodID::METHODTYPE_STATIC,
+                                     MethodID::EXCEPTIONCHECK_NO>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature, base::subtle::AtomicWord* atomic_method_id);
+
+template jmethodID MethodID::LazyGet<MethodID::METHODTYPE_NORMAL,
+                                     MethodID::EXCEPTIONCHECK_NO>(
+    JNIEnv* env, jclass clazz, const char* method_name,
+    const char* jni_signature, base::subtle::AtomicWord* atomic_method_id);
 
 jfieldID GetFieldID(JNIEnv* env,
                     const JavaRef<jclass>& clazz,
                     const char* field_name,
                     const char* jni_signature) {
   jfieldID field_id = env->GetFieldID(clazz.obj(), field_name, jni_signature);
   CHECK(!ClearException(env) && field_id) << "Failed to find field " <<
       field_name << " " << jni_signature;
   return field_id;
 }
@@ skipping 45 matching lines @@
     found = true;
   }
   base::subtle::Release_Store(&g_method_id_map_lock, kUnlocked);
 
   // Addition to the map does not invalidate this iterator.
   if (found) {
     return iter->second;
   }
 
   ScopedJavaLocalRef<jclass> clazz(env, env->FindClass(class_name));
-  jmethodID id = GetMethodID(env, clazz, method, jni_signature);
+  jmethodID id = MethodID::Get<
+      MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_YES>(
+          env, clazz.obj(), method, jni_signature);
 
   while (base::subtle::Acquire_CompareAndSwap(&g_method_id_map_lock,
                                               kUnlocked,
                                               kLocked) != kUnlocked) {
     base::PlatformThread::YieldCurrentThread();
   }
   // Another thread may have populated the map already.
   std::pair<MethodIDMap::const_iterator, bool> result =
       map->insert(std::make_pair(key, id));
   DCHECK_EQ(id, result.first->second);
@@ skipping 31 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