Don't use Singleton to cache JNI method IDs in Java Bridge

base/android/jni_android.cc

Index: base/android/jni_android.cc
diff --git a/base/android/jni_android.cc b/base/android/jni_android.cc
index e6d6e56c2677c29f046c600bc34b7f833b6b32a3..dd4049c11252d46e0b21e5443189cae53ac2bd2a 100644
--- a/base/android/jni_android.cc
+++ b/base/android/jni_android.cc
@@ -47,10 +47,12 @@ jobject GetApplicationContext() {
   return g_application_context;
 }
 
-MethodID::MethodID(JNIEnv* env, const char* class_name, const char* method,
-                   const char* jni_signature) {
+jmethodID GetMethodIDFromClassName(JNIEnv* env,
+                                   const char* class_name,
+                                   const char* method,
+                                   const char* jni_signature) {
   ScopedJavaLocalRef<jclass> clazz(env, env->FindClass(class_name));

[M-A Ruel, 2011/12/01 14:36:03]

If you implement caching at this layer, you'll have a single TLS LazyInstance
blocked with a SWMR lock. I've sent a thread to chromium-dev since I couldn't
find an implementation in our code (!)

Then you just do a lookup. I'd do something like:
std::string buffer(class_name):
buffer += "\x01";
buffer += method;
buffer += "\x01";
buffer += jni_signature;

Where "\x01" would be any invalid java symbol. Then you just need a simple
binary tree. If you expect the number of items to be low, use a std::vector and
use a binary_search, it'll be faster.

What do you think?

[Steve Block, 2011/12/01 17:48:47]

There's no need to cache JNIEnv (see above) and method IDs are constant and can
be shared across threads. So we can implement caching at this level with a
single threadsafe map.

-  id_ = GetMethodID(env, clazz.obj(), method, jni_signature);
+  return GetMethodID(env, clazz.obj(), method, jni_signature);
 }
 
 jmethodID GetMethodID(JNIEnv* env,