Bug 1157908 - Optimize pumpMessageLoop call to use less JNI; r=snorp
☠☠ backed out by 6d0fb655e2e6 ☠ ☠
authorJim Chen <nchen@mozilla.com>
Fri, 24 Apr 2015 14:40:55 -0400
changeset 240972 a486dcc9c23367a2c8642daa2f5b5c43e6b0429c
parent 240971 7164a2488b286057a748e0c412389b3665ecfd94
child 240973 98b0e8e0c1800f4771dc2629adf769327b3be965
push id58981
push usernchen@mozilla.com
push dateFri, 24 Apr 2015 18:42:02 +0000
treeherdermozilla-inbound@a486dcc9c233 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1157908
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1157908 - Optimize pumpMessageLoop call to use less JNI; r=snorp
mobile/android/base/GeckoAppShell.java
mobile/android/base/GeckoThread.java
mobile/android/base/util/ThreadUtils.java
mozglue/android/jni-stubs.inc
widget/android/AndroidBridge.cpp
widget/android/AndroidBridge.h
widget/android/AndroidJNI.cpp
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
widget/android/nsAppShell.cpp
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -249,27 +249,26 @@ public class GeckoAppShell
     static public final int LINK_TYPE_2G = 5;
     static public final int LINK_TYPE_3G = 6;
     static public final int LINK_TYPE_4G = 7;
 
     /* The Android-side API: API methods that Android calls */
 
     // Initialization methods
     public static native void registerJavaUiThread();
-    public static native void nativeInit(ClassLoader clsLoader);
+    public static native void nativeInit(ClassLoader clsLoader, MessageQueue msgQueue);
 
     // helper methods
     public static native void onResume();
     public static void callObserver(String observerKey, String topic, String data) {
         sendEventToGecko(GeckoEvent.createCallObserverEvent(observerKey, topic, data));
     }
     public static void removeObserver(String observerKey) {
         sendEventToGecko(GeckoEvent.createRemoveObserverEvent(observerKey));
     }
-    public static native Message getNextMessageFromQueue(MessageQueue queue);
     public static native void onSurfaceTextureFrameAvailable(Object surfaceTexture, int id);
     public static native void dispatchMemoryPressure();
 
     private static native void reportJavaCrash(String stackTrace);
 
     public static void notifyUriVisited(String uri) {
         sendEventToGecko(GeckoEvent.createVisitedEvent(uri));
     }
@@ -346,17 +345,17 @@ public class GeckoAppShell
                 geckoHandler.sendMessageAtFrontOfQueue(idleMsg);
                 // Keep this IdleHandler
                 return true;
             }
         };
         Looper.myQueue().addIdleHandler(idleHandler);
 
         // Initialize AndroidBridge.
-        nativeInit(GeckoAppShell.class.getClassLoader());
+        nativeInit(GeckoAppShell.class.getClassLoader(), Looper.myQueue());
 
         // First argument is the .apk path
         String combinedArgs = apkPath + " -greomni " + apkPath;
         if (args != null)
             combinedArgs += " " + args;
         if (url != null)
             combinedArgs += " -url " + url;
         if (type != null)
@@ -2427,23 +2426,18 @@ public class GeckoAppShell
     }
 
     @WrapElementForJNI
     public static void unlockScreenOrientation() {
         GeckoScreenOrientation.getInstance().unlock();
     }
 
     @WrapElementForJNI
-    public static boolean pumpMessageLoop() {
-        Handler geckoHandler = ThreadUtils.sGeckoHandler;
-        Message msg = getNextMessageFromQueue(ThreadUtils.sGeckoQueue);
-
-        if (msg == null) {
-            return false;
-        }
+    public static boolean pumpMessageLoop(final Message msg) {
+        final Handler geckoHandler = ThreadUtils.sGeckoHandler;
 
         if (msg.obj == geckoHandler && msg.getTarget() == geckoHandler) {
             // Our "queue is empty" message; see runGecko()
             return false;
         }
 
         if (msg.getTarget() == null) {
             Looper.myLooper().quit();
--- a/mobile/android/base/GeckoThread.java
+++ b/mobile/android/base/GeckoThread.java
@@ -151,17 +151,16 @@ public class GeckoThread extends Thread 
         return (args != null ? args : "") + profileArg + guestArg;
     }
 
     @Override
     public void run() {
         Looper.prepare();
         ThreadUtils.sGeckoThread = this;
         ThreadUtils.sGeckoHandler = new Handler();
-        ThreadUtils.sGeckoQueue = Looper.myQueue();
 
         if (mDebugging) {
             try {
                 Thread.sleep(5 * 1000 /* 5 seconds */);
             } catch (final InterruptedException e) {
             }
         }
 
--- a/mobile/android/base/util/ThreadUtils.java
+++ b/mobile/android/base/util/ThreadUtils.java
@@ -31,17 +31,16 @@ public final class ThreadUtils {
 
     private static volatile Thread sBackgroundThread;
 
     // Referenced directly from GeckoAppShell in highly performance-sensitive code (The extra
     // function call of the getter was harming performance. (Bug 897123))
     // Once Bug 709230 is resolved we should reconsider this as ProGuard should be able to optimise
     // this out at compile time.
     public static Handler sGeckoHandler;
-    public static MessageQueue sGeckoQueue;
     public static volatile Thread sGeckoThread;
 
     // Delayed Runnable that resets the Gecko thread priority.
     private static final Runnable sPriorityResetRunnable = new Runnable() {
         @Override
         public void run() {
             resetGeckoPriority();
         }
--- a/mozglue/android/jni-stubs.inc
+++ b/mozglue/android/jni-stubs.inc
@@ -72,26 +72,26 @@ Java_org_mozilla_gecko_GeckoAppShell_reg
 #endif
 
 #ifdef JNI_BINDINGS
   xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_registerJavaUiThread", &f_Java_org_mozilla_gecko_GeckoAppShell_registerJavaUiThread);
 #endif
 
 #ifdef JNI_STUBS
 
-typedef void (*Java_org_mozilla_gecko_GeckoAppShell_nativeInit_t)(JNIEnv *, jclass, jobject);
+typedef void (*Java_org_mozilla_gecko_GeckoAppShell_nativeInit_t)(JNIEnv *, jclass, jobject, jobject);
 static Java_org_mozilla_gecko_GeckoAppShell_nativeInit_t f_Java_org_mozilla_gecko_GeckoAppShell_nativeInit;
 extern "C" NS_EXPORT void JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_nativeInit(JNIEnv * arg0, jclass arg1, jobject arg2) {
+Java_org_mozilla_gecko_GeckoAppShell_nativeInit(JNIEnv * arg0, jclass arg1, jobject arg2, jobject arg3) {
     if (!f_Java_org_mozilla_gecko_GeckoAppShell_nativeInit) {
         arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
                        "JNI Function called before it was loaded");
         return ;
     }
-     f_Java_org_mozilla_gecko_GeckoAppShell_nativeInit(arg0, arg1, arg2);
+     f_Java_org_mozilla_gecko_GeckoAppShell_nativeInit(arg0, arg1, arg2, arg3);
 }
 #endif
 
 #ifdef JNI_BINDINGS
   xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_nativeInit", &f_Java_org_mozilla_gecko_GeckoAppShell_nativeInit);
 #endif
 
 #ifdef JNI_STUBS
@@ -110,35 +110,16 @@ Java_org_mozilla_gecko_GeckoAppShell_onR
 #endif
 
 #ifdef JNI_BINDINGS
   xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_onResume", &f_Java_org_mozilla_gecko_GeckoAppShell_onResume);
 #endif
 
 #ifdef JNI_STUBS
 
-typedef jobject (*Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue_t)(JNIEnv *, jclass, jobject);
-static Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue_t f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue;
-extern "C" NS_EXPORT jobject JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue(JNIEnv * arg0, jclass arg1, jobject arg2) {
-    if (!f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue) {
-        arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
-                       "JNI Function called before it was loaded");
-        return nullptr;
-    }
-    return f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue(arg0, arg1, arg2);
-}
-#endif
-
-#ifdef JNI_BINDINGS
-  xul_dlsym("Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue", &f_Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue);
-#endif
-
-#ifdef JNI_STUBS
-
 typedef void (*Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable_t)(JNIEnv *, jclass, jobject, jint);
 static Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable_t f_Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable;
 extern "C" NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable(JNIEnv * arg0, jclass arg1, jobject arg2, jint arg3) {
     if (!f_Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable) {
         arg0->ThrowNew(arg0->FindClass("java/lang/UnsupportedOperationException"),
                        "JNI Function called before it was loaded");
         return ;
--- a/widget/android/AndroidBridge.cpp
+++ b/widget/android/AndroidBridge.cpp
@@ -145,31 +145,41 @@ jfieldID AndroidBridge::GetStaticFieldID
              fieldName, fieldType);
         env->ExceptionDescribe();
         MOZ_CRASH();
     }
     return fieldID;
 }
 
 void
-AndroidBridge::ConstructBridge(JNIEnv *jEnv, Object::Param clsLoader)
+AndroidBridge::ConstructBridge(JNIEnv *jEnv, Object::Param clsLoader, Object::Param msgQueue)
 {
     /* NSS hack -- bionic doesn't handle recursive unloads correctly,
      * because library finalizer functions are called with the dynamic
      * linker lock still held.  This results in a deadlock when trying
      * to call dlclose() while we're already inside dlclose().
      * Conveniently, NSS has an env var that can prevent it from unloading.
      */
     putenv("NSS_DISABLE_UNLOAD=1");
 
     PR_NewThreadPrivateIndex(&sJavaEnvThreadIndex, JavaThreadDetachFunc);
 
     MOZ_ASSERT(!sBridge);
     sBridge = new AndroidBridge;
     sBridge->Init(jEnv, clsLoader); // Success or crash
+
+    auto msgQueueClass = ClassObject::LocalRef::Adopt(
+            jEnv, jEnv->GetObjectClass(msgQueue.Get()));
+    sBridge->mMessageQueue = msgQueue;
+    // mMessageQueueNext must not be null
+    sBridge->mMessageQueueNext = GetMethodID(
+            jEnv, msgQueueClass.Get(), "next", "()Landroid/os/Message;");
+    // mMessageQueueMessages may be null (e.g. due to proguard optimization)
+    sBridge->mMessageQueueMessages = jEnv->GetFieldID(
+            msgQueueClass.Get(), "mMessages", "Landroid/os/Message;");
 }
 
 void
 AndroidBridge::Init(JNIEnv *jEnv, Object::Param clsLoader)
 {
     ALOG_BRIDGE("AndroidBridge::Init");
     jEnv->GetJavaVM(&mJavaVM);
     if (!mJavaVM) {
@@ -1661,16 +1671,42 @@ AndroidBridge::GetProxyForURI(const nsAC
 
     if (!jstrRet)
         return NS_ERROR_FAILURE;
 
     aResult = nsCString(jstrRet);
     return NS_OK;
 }
 
+bool
+AndroidBridge::PumpMessageLoop()
+{
+    JNIEnv* const env = GetJNIEnv();
+
+    if (mMessageQueueMessages) {
+        auto msg = Object::LocalRef::Adopt(env,
+                env->GetObjectField(mMessageQueue.Get(),
+                                    mMessageQueueMessages));
+        // if queue.mMessages is null, queue.next() will block, which we don't
+        // want. It turns out to be an order of magnitude more performant to do
+        // this extra check here and block less vs. one fewer checks here and
+        // more blocking.
+        if (!msg) {
+            return false;
+        }
+    }
+
+    auto msg = Object::LocalRef::Adopt(
+            env, env->CallObjectMethod(mMessageQueue.Get(), mMessageQueueNext));
+    if (!msg) {
+        return false;
+    }
+
+    return GeckoAppShell::PumpMessageLoop(msg);
+}
 
 /* attribute nsIAndroidBrowserApp browserApp; */
 NS_IMETHODIMP nsAndroidBridge::GetBrowserApp(nsIAndroidBrowserApp * *aBrowserApp)
 {
     if (nsAppShell::gAppShell)
         nsAppShell::gAppShell->GetBrowserApp(aBrowserApp);
     return NS_OK;
 }
--- a/widget/android/AndroidBridge.h
+++ b/widget/android/AndroidBridge.h
@@ -128,17 +128,19 @@ public:
     static void RegisterJavaUiThread() {
         sJavaUiThread = pthread_self();
     }
 
     static bool IsJavaUiThread() {
         return pthread_equal(pthread_self(), sJavaUiThread);
     }
 
-    static void ConstructBridge(JNIEnv *jEnv, jni::Object::Param clsLoader);
+    static void ConstructBridge(JNIEnv *jEnv,
+                                jni::Object::Param clsLoader,
+                                jni::Object::Param msgQueue);
 
     static AndroidBridge *Bridge() {
         return sBridge;
     }
 
     static JavaVM *GetVM() {
         MOZ_ASSERT(sBridge);
         return sBridge->mJavaVM;
@@ -309,16 +311,18 @@ public:
     void ScheduleComposite();
 
     nsresult GetProxyForURI(const nsACString & aSpec,
                             const nsACString & aScheme,
                             const nsACString & aHost,
                             const int32_t      aPort,
                             nsACString & aResult);
 
+    bool PumpMessageLoop();
+
     // Utility methods.
     static jstring NewJavaString(JNIEnv* env, const char16_t* string, uint32_t len);
     static jstring NewJavaString(JNIEnv* env, const nsAString& string);
     static jstring NewJavaString(JNIEnv* env, const char* string);
     static jstring NewJavaString(JNIEnv* env, const nsACString& string);
 
     static jstring NewJavaString(AutoLocalJNIFrame* frame, const char16_t* string, uint32_t len);
     static jstring NewJavaString(AutoLocalJNIFrame* frame, const nsAString& string);
@@ -405,16 +409,20 @@ protected:
     widget::GLController::GlobalRef mGLControllerObj;
 
     // some convinient types to have around
     jclass jStringClass;
 
     jni::Object::GlobalRef mClassLoader;
     jmethodID mClassLoaderLoadClass;
 
+    jni::Object::GlobalRef mMessageQueue;
+    jfieldID mMessageQueueMessages;
+    jmethodID mMessageQueueNext;
+
     // calls we've dlopened from libjnigraphics.so
     int (* AndroidBitmap_getInfo)(JNIEnv *env, jobject bitmap, void *info);
     int (* AndroidBitmap_lockPixels)(JNIEnv *env, jobject bitmap, void **buffer);
     int (* AndroidBitmap_unlockPixels)(JNIEnv *env, jobject bitmap);
 
     void* (*ANativeWindow_fromSurface)(JNIEnv *env, jobject surface);
     void* (*ANativeWindow_fromSurfaceTexture)(JNIEnv *env, jobject surfaceTexture);
     void (*ANativeWindow_release)(void *window);
--- a/widget/android/AndroidJNI.cpp
+++ b/widget/android/AndroidJNI.cpp
@@ -60,19 +60,20 @@ extern "C" {
 
 NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_registerJavaUiThread(JNIEnv *jenv, jclass jc)
 {
     AndroidBridge::RegisterJavaUiThread();
 }
 
 NS_EXPORT void JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_nativeInit(JNIEnv *jenv, jclass, jobject clsLoader)
+Java_org_mozilla_gecko_GeckoAppShell_nativeInit(JNIEnv *jenv, jclass, jobject clsLoader, jobject msgQueue)
 {
-    AndroidBridge::ConstructBridge(jenv, jni::Object::Ref::From(clsLoader));
+    AndroidBridge::ConstructBridge(
+            jenv, jni::Object::Ref::From(clsLoader), jni::Object::Ref::From(msgQueue));
 }
 
 NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_notifyGeckoOfEvent(JNIEnv *jenv, jclass jc, jobject event)
 {
     // poke the appshell
     if (nsAppShell::gAppShell)
         nsAppShell::gAppShell->PostEvent(AndroidGeckoEvent::MakeFromJavaObject(jenv, event));
@@ -849,43 +850,16 @@ Java_org_mozilla_gecko_GeckoAppShell_onF
     private:
       jobject mView;
   };
 
   nsCOMPtr<nsIRunnable> runnable = new ExitFullScreenRunnable(jenv->NewGlobalRef(view));
   NS_DispatchToMainThread(runnable);
 }
 
-NS_EXPORT jobject JNICALL
-Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue(JNIEnv* jenv, jclass, jobject queue)
-{
-    static jclass jMessageQueueCls = nullptr;
-    static jfieldID jMessagesField;
-    static jmethodID jNextMethod;
-    if (!jMessageQueueCls) {
-        jMessageQueueCls = (jclass) jenv->NewGlobalRef(jenv->FindClass("android/os/MessageQueue"));
-        jNextMethod = jenv->GetMethodID(jMessageQueueCls, "next", "()Landroid/os/Message;");
-        jMessagesField = jenv->GetFieldID(jMessageQueueCls, "mMessages", "Landroid/os/Message;");
-    }
-
-    if (!jMessageQueueCls || !jNextMethod)
-        return nullptr;
-
-    if (jMessagesField) {
-        jobject msg = jenv->GetObjectField(queue, jMessagesField);
-        // if queue.mMessages is null, queue.next() will block, which we don't want
-        // It turns out to be an order of magnitude more performant to do this extra check here and
-        // block less vs. one fewer checks here and more blocking.
-        if (!msg) {
-            return nullptr;
-        }
-    }
-    return jenv->CallObjectMethod(queue, jNextMethod);
-}
-
 NS_EXPORT void JNICALL
 Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable(JNIEnv* jenv, jclass, jobject surfaceTexture, jint id)
 {
   mozilla::gl::AndroidSurfaceTexture* st = mozilla::gl::AndroidSurfaceTexture::Find(id);
   if (!st) {
     __android_log_print(ANDROID_LOG_ERROR, "GeckoJNI", "Failed to find AndroidSurfaceTexture with id %d", id);
     return;
   }
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -524,19 +524,19 @@ constexpr char GeckoAppShell::PerformHap
 void GeckoAppShell::PerformHapticFeedback(bool a0)
 {
     return mozilla::jni::Method<PerformHapticFeedback_t>::Call(nullptr, nullptr, a0);
 }
 
 constexpr char GeckoAppShell::PumpMessageLoop_t::name[];
 constexpr char GeckoAppShell::PumpMessageLoop_t::signature[];
 
-bool GeckoAppShell::PumpMessageLoop()
+bool GeckoAppShell::PumpMessageLoop(mozilla::jni::Object::Param a0)
 {
-    return mozilla::jni::Method<PumpMessageLoop_t>::Call(nullptr, nullptr);
+    return mozilla::jni::Method<PumpMessageLoop_t>::Call(nullptr, nullptr, a0);
 }
 
 constexpr char GeckoAppShell::RegisterSurfaceTextureFrameListener_t::name[];
 constexpr char GeckoAppShell::RegisterSurfaceTextureFrameListener_t::signature[];
 
 void GeckoAppShell::RegisterSurfaceTextureFrameListener(mozilla::jni::Object::Param a0, int32_t a1)
 {
     return mozilla::jni::Method<RegisterSurfaceTextureFrameListener_t>::Call(nullptr, nullptr, a0, a1);
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -1004,23 +1004,23 @@ public:
 
 public:
     struct PumpMessageLoop_t {
         typedef GeckoAppShell Owner;
         typedef bool ReturnType;
         typedef bool SetterType;
         static constexpr char name[] = "pumpMessageLoop";
         static constexpr char signature[] =
-                "()Z";
+                "(Landroid/os/Message;)Z";
         static const bool isStatic = true;
         static const bool isMultithreaded = false;
         static const mozilla::jni::ExceptionMode exceptionMode = mozilla::jni::ExceptionMode::ABORT;
     };
 
-    static bool PumpMessageLoop();
+    static bool PumpMessageLoop(mozilla::jni::Object::Param);
 
 public:
     struct RegisterSurfaceTextureFrameListener_t {
         typedef GeckoAppShell Owner;
         typedef void ReturnType;
         typedef void SetterType;
         static constexpr char name[] = "registerSurfaceTextureFrameListener";
         static constexpr char signature[] =
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -243,17 +243,17 @@ nsAppShell::ProcessNextNativeEvent(bool 
 
         curEvent = PopNextEvent();
         if (!curEvent && mayWait) {
             // This processes messages in the Android Looper. Note that we only
             // get here if the normal Gecko event loop has been awoken
             // (bug 750713). Looper messages effectively have the lowest
             // priority because we only process them before we're about to
             // wait for new events.
-            if (widget::GeckoAppShell::PumpMessageLoop()) {
+            if (AndroidBridge::Bridge()->PumpMessageLoop()) {
                 return true;
             }
 
             PROFILER_LABEL("nsAppShell", "ProcessNextNativeEvent::Wait",
                 js::ProfileEntry::Category::EVENTS);
             mozilla::HangMonitor::Suspend();
 
             // hmm, should we really hardcode this 10s?