Bug 844275 - Move the GfxInfoThread so that it is guaranteed to run to completion before compositor creation. r=Cwiiis a=bajaj
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 18 Mar 2013 23:00:04 +0100
changeset 132419 e8c1062ccaf0e5e8fd2b91d0b16415edec2c62e6
parent 132418 314d4dd7dc737134d03e31a43464b92e8fdd58a9
child 132420 5aa5a4e399e13ae458cf56ba140275ff5346107b
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCwiiis, bajaj
bugs844275
milestone21.0a2
Bug 844275 - Move the GfxInfoThread so that it is guaranteed to run to completion before compositor creation. r=Cwiiis a=bajaj
mobile/android/base/GeckoAppShell.java
mobile/android/base/GeckoThread.java
mobile/android/base/gfx/GLController.java
mobile/android/base/gfx/GfxInfoThread.java
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -144,18 +144,16 @@ public class GeckoAppShell
     private static Sensor gOrientationSensor = null;
     private static Sensor gProximitySensor = null;
     private static Sensor gLightSensor = null;
 
     private static boolean mLocationHighAccuracy = false;
 
     private static Handler sGeckoHandler;
 
-    public static GfxInfoThread sGfxInfoThread = null;
-
     static ActivityHandlerHelper sActivityHelper = new ActivityHandlerHelper();
     static NotificationServiceClient sNotificationClient;
 
     /* The Android-side API: API methods that Android calls */
 
     // Initialization methods
     public static native void nativeInit();
 
@@ -2095,19 +2093,17 @@ public class GeckoAppShell
         }
     }
 
     public static void notifyWakeLockChanged(String topic, String state) {
         GeckoApp.mAppContext.notifyWakeLockChanged(topic, state);
     }
 
     public static String getGfxInfoData() {
-        String data = sGfxInfoThread.getData();
-        sGfxInfoThread = null;
-        return data;
+        return GfxInfoThread.getData();
     }
 
     public static void registerSurfaceTextureFrameListener(Object surfaceTexture, final int id) {
         ((SurfaceTexture)surfaceTexture).setOnFrameAvailableListener(new SurfaceTexture.OnFrameAvailableListener() {
             public void onFrameAvailable(SurfaceTexture surfaceTexture) {
                 GeckoAppShell.onSurfaceTextureFrameAvailable(surfaceTexture, id);
             }
         });
--- a/mobile/android/base/GeckoThread.java
+++ b/mobile/android/base/GeckoThread.java
@@ -1,16 +1,15 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
-import org.mozilla.gecko.gfx.GfxInfoThread;
 import org.mozilla.gecko.mozglue.GeckoLoader;
 import org.mozilla.gecko.util.GeckoEventListener;
 
 import org.json.JSONObject;
 
 import android.content.Intent;
 import android.content.res.Configuration;
 import android.content.res.Resources;
@@ -86,25 +85,16 @@ public class GeckoThread extends Thread 
     }
 
     private String addCustomProfileArg(String args) {
         String profile = GeckoApp.sIsUsingCustomProfile ? "" : (" -P " + GeckoApp.mAppContext.getProfile().getName());
         return (args != null ? args : "") + profile;
     }
 
     public void run() {
-        // Here we start the GfxInfo thread, which will query OpenGL
-        // system information for Gecko. This must be done early enough that the data will be
-        // ready by the time it's needed to initialize the LayerManager (it takes about 100 ms
-        // to obtain). Doing it here seems to have no negative effect on startup time. See bug 766251.
-        // Starting the GfxInfoThread here from the GeckoThread, ensures that
-        // the Gecko thread is started first, adding some determinism there.
-        GeckoAppShell.sGfxInfoThread = new GfxInfoThread();
-        GeckoAppShell.sGfxInfoThread.start();
-
         String path = initGeckoEnvironment();
 
         Log.w(LOGTAG, "zerdatime " + SystemClock.uptimeMillis() + " - runGecko");
 
         String args = addCustomProfileArg(mIntent.getStringExtra("args"));
         String type = getTypeFromAction(mIntent.getAction());
         mIntent = null;
 
--- a/mobile/android/base/gfx/GLController.java
+++ b/mobile/android/base/gfx/GLController.java
@@ -54,16 +54,21 @@ public class GLController {
         EGL10.EGL_GREEN_SIZE, 6,
         EGL10.EGL_BLUE_SIZE, 5,
         EGL10.EGL_SURFACE_TYPE, EGL10.EGL_WINDOW_BIT,
         EGL10.EGL_RENDERABLE_TYPE, LOCAL_EGL_OPENGL_ES2_BIT,
         EGL10.EGL_NONE
     };
 
     private GLController() {
+        // Here we start the GfxInfo thread, which will query OpenGL
+        // system information for Gecko. This must be done early enough that the data will be
+        // ready by the time it's needed to initialize the compositor (it takes about 100 ms
+        // to obtain).
+        GfxInfoThread.startThread();
     }
 
     static GLController getInstance(LayerView view) {
         if (sInstance == null) {
             sInstance = new GLController();
         }
         sInstance.mView = view;
         return sInstance;
@@ -110,16 +115,27 @@ public class GLController {
         // done setting up - for some reason Android will send us a surfaceChanged
         // notification before the surface is actually ready. So, we need to do the
         // call to eglCreateWindowSurface in a runnable posted back to the UI thread
         // that will run once this call unwinds all the way out and Android finishes
         // doing its thing.
 
         mView.post(new Runnable() {
             public void run() {
+                // If we haven't yet created the compositor, and the GfxInfoThread
+                // isn't done it's data gathering activities, then postpone creating
+                // the compositor a little bit more. Don't block though, since this is
+                // the UI thread we're running on. This conditional also ensures that
+                // we don't call GfxInfoThread.hasData() once we have created the
+                // compositor, as that is not allowed (see GfxInfoThread).
+                if (!mCompositorCreated && !GfxInfoThread.hasData()) {
+                    mView.postDelayed(this, 1);
+                    return;
+                }
+
                 try {
                     // Re-check mSurfaceValid in case the surface was destroyed between
                     // where we set it to true above and this runnable getting run.
                     // If mSurfaceValid is still true, try to create mEGLSurface. If
                     // mSurfaceValid is false, leave mEGLSurface as null. So at the end
                     // of this block mEGLSurface will be null (or EGL_NO_SURFACE) if
                     // eglCreateWindowSurface failed or if mSurfaceValid changed to false.
                     if (mSurfaceValid) {
--- a/mobile/android/base/gfx/GfxInfoThread.java
+++ b/mobile/android/base/gfx/GfxInfoThread.java
@@ -12,55 +12,76 @@ import java.util.concurrent.SynchronousQ
 
 import javax.microedition.khronos.egl.EGL10;
 import javax.microedition.khronos.egl.EGLConfig;
 import javax.microedition.khronos.egl.EGLContext;
 import javax.microedition.khronos.egl.EGLDisplay;
 import javax.microedition.khronos.egl.EGLSurface;
 
 public class GfxInfoThread extends Thread {
-
     private static final String LOGTAG = "GfxInfoThread";
 
-    private SynchronousQueue<String> mDataQueue;
+    private static GfxInfoThread sInstance;
+    private String mData;
+
+    private GfxInfoThread() {
+    }
 
-    public GfxInfoThread() {
-        mDataQueue = new SynchronousQueue<String>();
+    public static void startThread() {
+        if (sInstance == null) {
+            sInstance = new GfxInfoThread();
+            sInstance.start();
+        }
     }
 
-    private void error(String msg) {
+    public static boolean hasData() {
+        // This should never be called before startThread() or after getData()
+        // so we know sInstance will be non-null here
+        synchronized (sInstance) {
+            return sInstance.mData != null;
+        }
+    }
+
+    public static String getData() {
+        // This should be called exactly once after startThread(), so we
+        // know sInstance will be non-null here
+        String data = sInstance.getDataImpl();
+        sInstance = null;
+        return data;
+    }
+
+    private synchronized void error(String msg) {
         Log.e(LOGTAG, msg);
-        try {
-            mDataQueue.put("ERROR\n" + msg + "\n");
-        } catch (InterruptedException e) {
-            Log.w(LOGTAG, "Thread interrupted", e);
-            Thread.currentThread().interrupt();
-        }
+        mData = "ERROR\n" + msg + "\n";
+        notifyAll();
     }
 
     private void eglError(EGL10 egl, String msg) {
         error(msg + " (EGL error " + Integer.toHexString(egl.eglGetError()) + ")");
     }
 
-    public String getData() {
-        String data = mDataQueue.poll();
-        if (data != null)
-            return data;
+    private synchronized String getDataImpl() {
+        if (mData != null) {
+            return mData;
+        }
 
         Log.w(LOGTAG, "We need the GfxInfo data, but it is not yet available. " +
                       "We have to wait for it, so expect abnormally long startup times. " +
                       "Please report a Mozilla bug.");
+
         try {
-            data = mDataQueue.take();
+            while (mData == null) {
+                wait();
+            }
         } catch (InterruptedException e) {
             Log.w(LOGTAG, "Thread interrupted", e);
             Thread.currentThread().interrupt();
         }
         Log.i(LOGTAG, "GfxInfo data is finally available.");
-        return data;
+        return mData;
     }
 
     public void run() {
         // initialize EGL
         EGL10 egl = (EGL10) EGLContext.getEGL();
         EGLDisplay eglDisplay = egl.eglGetDisplay(EGL10.EGL_DEFAULT_DISPLAY);
 
         if (eglDisplay == EGL10.EGL_NO_DISPLAY) {
@@ -177,16 +198,14 @@ public class GfxInfoThread extends Threa
         // have a very low limit on the global number of GL contexts.
         egl.eglMakeCurrent(eglDisplay, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_CONTEXT);
         egl.eglDestroySurface(eglDisplay, eglSurface);
         egl.eglDestroyContext(eglDisplay, eglContext);
         // intentionally do not eglTerminate: maybe this will make the next eglInitialize faster?
 
         // finally send the data. Notice that we've already freed the EGL resources, so that they don't
         // remain there until the data is read.
-        try {
-            mDataQueue.put(data);
-        } catch (InterruptedException e) {
-            Log.w(LOGTAG, "Thread interrupted", e);
-            Thread.currentThread().interrupt();
+        synchronized (this) {
+            mData = data;
+            notifyAll();
         }
     }
 }