Merge pull request #672 from Dexterp37/faster_init

Make Glean init in Kotlin fully async

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 * General:
   * `ping_type` is not included in the `ping_info` any more ([#653](https://github.com/mozilla/glean/pull/653)), the pipeline takes the value from the submission URL.
+* Android:
+  * The `Glean.initialize` method runs mostly off the main thread ([#672](https://github.com/mozilla/glean/pull/672)).
 * iOS:
   * The baseline ping will now include `reason` codes that indicate why it was
     submitted. If an unclean shutdown is detected (e.g. due to force-close), this

glean-core/android/src/main/java/mozilla/telemetry/glean/Dispatchers.kt

@@ -107,7 +107,7 @@ internal object Dispatchers {
          * set to false prior to processing the queue, newly launched tasks should be executed
          * on the couroutine scope rather than added to the queue.
          */
-        internal fun flushQueuedInitialTasks() {
+        internal suspend fun flushQueuedInitialTasks() {
             val dispatcherObject = this
             // Dispatch a task to flush the pre-init tasks queue. By using `executeTask`
             // this will be executed as soon as possible, before other tasks are executed.
@@ -139,7 +139,7 @@ internal object Dispatchers {
                 if (overflowCount > 0) {
                     GleanError.preinitTasksOverflow.addSync(MAX_QUEUE_SIZE + overflowCount)
                 }
-            }
+            }?.join()
         }
 
         /**

glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt

@@ -15,6 +15,8 @@ import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.ProcessLifecycleOwner
 import com.sun.jna.StringArray
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.MainScope
+import kotlinx.coroutines.launch
 import mozilla.telemetry.glean.config.Configuration
 import mozilla.telemetry.glean.config.FfiConfiguration
 import mozilla.telemetry.glean.utils.getLocaleTag
@@ -135,95 +137,86 @@ open class GleanInternalAPI internal constructor () {
             return
         }
 
-        setUploadEnabled(uploadEnabled)
-
-        registerPings(Pings)
-
         this.applicationContext = applicationContext
 
         this.configuration = configuration
         this.gleanDataDir = File(applicationContext.applicationInfo.dataDir, GLEAN_DATA_DIR)
 
-        val cfg = FfiConfiguration(
-            dataDir = this.gleanDataDir.path,
-            packageName = applicationContext.packageName,
-            uploadEnabled = uploadEnabled,
-            maxEvents = this.configuration.maxEvents,
-            delayPingLifetimeIO = false
-        )
+        setUploadEnabled(uploadEnabled)
 
-        initialized = LibGleanFFI.INSTANCE.glean_initialize(cfg).toBoolean()
+        // Execute startup off the main thread.
+        @Suppress("EXPERIMENTAL_API_USAGE")
+        Dispatchers.API.executeTask {
+            registerPings(Pings)
+
+            val cfg = FfiConfiguration(
+                dataDir = gleanDataDir.path,
+                packageName = applicationContext.packageName,
+                uploadEnabled = uploadEnabled,
+                maxEvents = configuration.maxEvents,
+                delayPingLifetimeIO = false
+            )
 
-        // If initialization of Glean fails we bail out and don't initialize further.
-        if (!initialized) {
-            return
-        }
+            initialized = LibGleanFFI.INSTANCE.glean_initialize(cfg).toBoolean()
 
-        // If any pings were registered before initializing, do so now.
-        // We're not clearing this queue in case Glean is reset by tests.
-        this.pingTypeQueue.forEach { this.registerPingType(it) }
+            // If initialization of Glean fails we bail out and don't initialize further.
+            if (!initialized) {
+                return@executeTask
+            }
 
-        // If this is the first time ever the Glean SDK runs, make sure to set
-        // some initial core metrics in case we need to generate early pings.
-        // The next times we start, we would have them around already.
-        val isFirstRun = LibGleanFFI.INSTANCE.glean_is_first_run().toBoolean()
-        if (isFirstRun) {
-            initializeCoreMetrics(applicationContext)
-        }
+            // If any pings were registered before initializing, do so now.
+            // We're not clearing this queue in case Glean is reset by tests.
+            pingTypeQueue.forEach { registerPingType(it) }
 
-        // Deal with any pending events so we can start recording new ones
-        @Suppress("EXPERIMENTAL_API_USAGE")
-        Dispatchers.API.executeTask {
+            // If this is the first time ever the Glean SDK runs, make sure to set
+            // some initial core metrics in case we need to generate early pings.
+            // The next times we start, we would have them around already.
+            val isFirstRun = LibGleanFFI.INSTANCE.glean_is_first_run().toBoolean()
+            if (isFirstRun) {
+                initializeCoreMetrics(applicationContext)
+            }
+
+            // Deal with any pending events so we can start recording new ones
             val pingSubmitted = LibGleanFFI.INSTANCE.glean_on_ready_to_submit_pings().toBoolean()
             if (pingSubmitted) {
                 PingUploadWorker.enqueueWorker(applicationContext)
             }
-        }
 
-        // Set up information and scheduling for Glean owned pings. Ideally, the "metrics"
-        // ping startup check should be performed before any other ping, since it relies
-        // on being dispatched to the API context before any other metric.
-        metricsPingScheduler = MetricsPingScheduler(applicationContext)
-        metricsPingScheduler.schedule()
+            // Set up information and scheduling for Glean owned pings. Ideally, the "metrics"
+            // ping startup check should be performed before any other ping, since it relies
+            // on being dispatched to the API context before any other metric.
+            metricsPingScheduler = MetricsPingScheduler(applicationContext)
+            metricsPingScheduler.schedule()
 
-        // From the second time we run, after all startup pings are generated,
-        // make sure to clear `lifetime: application` metrics and set them again.
-        // Any new value will be sent in newly generted pings after startup.
-        if (!isFirstRun) {
-            @Suppress("EXPERIMENTAL_API_USAGE")
-            Dispatchers.API.executeTask {
+            // From the second time we run, after all startup pings are generated,
+            // make sure to clear `lifetime: application` metrics and set them again.
+            // Any new value will be sent in newly generated pings after startup.
+            if (!isFirstRun) {
                 LibGleanFFI.INSTANCE.glean_clear_application_lifetime_metrics()
                 initializeCoreMetrics(applicationContext)
             }
-        }
-
-        // Signal Dispatcher that init is complete
-        @Suppress("EXPERIMENTAL_API_USAGE")
-        Dispatchers.API.flushQueuedInitialTasks()
 
-        // Check if the "dirty flag" is set. That means the product was probably
-        // force-closed. If that's the case, submit a 'baseline' ping with the
-        // reason "dirty_startup". We only do that from the second run.
-        if (!isFirstRun) {
-            @Suppress("EXPERIMENTAL_API_USAGE")
-            Dispatchers.API.launch {
-                if (LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean()) {
-                    submitPingByNameSync("baseline", "dirty_startup")
-                    // Note: while in theory we should set the "dirty flag" to true
-                    // here, in practice it's not needed: if it hits this branch, it
-                    // means the value was `true` and nothing needs to be done.
-                }
+            // Signal Dispatcher that init is complete
+            Dispatchers.API.flushQueuedInitialTasks()
+
+            // Check if the "dirty flag" is set. That means the product was probably
+            // force-closed. If that's the case, submit a 'baseline' ping with the
+            // reason "dirty_startup". We only do that from the second run.
+            if (!isFirstRun && LibGleanFFI.INSTANCE.glean_is_dirty_flag_set().toBoolean()) {
+                submitPingByNameSync("baseline", "dirty_startup")
+                // Note: while in theory we should set the "dirty flag" to true
+                // here, in practice it's not needed: if it hits this branch, it
+                // means the value was `true` and nothing needs to be done.
             }
-        }
 
-        // At this point, all metrics and events can be recorded.
-        // This should only be called from the main thread. This is enforced by
-        // the @MainThread decorator and the `assertOnUiThread` call.
-        ProcessLifecycleOwner.get().lifecycle.addObserver(gleanLifecycleObserver)
+            // At this point, all metrics and events can be recorded.
+            // This should only be called from the main thread. This is enforced by
+            // the @MainThread decorator and the `assertOnUiThread` call.
+            MainScope().launch {
+                ProcessLifecycleOwner.get().lifecycle.addObserver(gleanLifecycleObserver)
+            }
 
-        if (!uploadEnabled) {
-            @Suppress("EXPERIMENTAL_API_USAGE")
-            Dispatchers.API.launch {
+            if (!uploadEnabled) {
                 DeletionPingUploadWorker.enqueueWorker(applicationContext)
             }
         }

glean-core/android/src/main/java/mozilla/telemetry/glean/utils/WorkManagerUtils.kt

@@ -8,6 +8,7 @@ import android.content.Context
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import androidx.work.testing.WorkManagerTestInitHelper
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
 
@@ -35,7 +36,7 @@ internal fun testFlushWorkManagerJob(context: Context, workTag: String, timeoutM
                         return@withTimeout
                     }
                 }
-            } while (true)
+            } while (isActive)
         }
     }
 }

glean-core/android/src/test/java/mozilla/telemetry/glean/DispatchersTest.kt

@@ -5,9 +5,11 @@
 package mozilla.telemetry.glean
 
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.Dispatchers as KotlinDispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.joinAll
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
@@ -63,7 +65,9 @@ class DispatchersTest {
         assertEquals("Tasks have not run while in queue", 0, threadCanary)
 
         // Now trigger execution to ensure the tasks fired
-        Dispatchers.API.flushQueuedInitialTasks()
+        runBlocking {
+            Dispatchers.API.flushQueuedInitialTasks()
+        }
 
         assertEquals("Tasks have executed", 3, threadCanary)
         assertEquals("Task queue is cleared", 0, Dispatchers.API.taskQueue.size)
@@ -97,12 +101,14 @@ class DispatchersTest {
         assertEquals("Tasks have not run while in queue", 0, threadCanary.get())
 
         // Now trigger execution to ensure the tasks fired
-        Dispatchers.API.flushQueuedInitialTasks()
+        GlobalScope.launch {
+            Dispatchers.API.flushQueuedInitialTasks()
+        }
 
         // Wait for the flushed tasks to be executed.
         runBlocking {
             withTimeoutOrNull(2000) {
-                while (threadCanary.get() != 3) {
+                while (isActive && (threadCanary.get() != 3 || Dispatchers.API.taskQueue.size > 0)) {
                     delay(1)
                 }
             } ?: assertTrue("Timed out waiting for tasks to execute", false)
@@ -152,7 +158,7 @@ class DispatchersTest {
         runBlocking {
             // Ensure that all the required jobs have been added to the list.
             withTimeoutOrNull(2000) {
-                while (counter.get() < 100) {
+                while (isActive && counter.get() < 100) {
                     delay(1)
                 }
             } ?: assertEquals("Timed out waiting for tasks to execute", 100, counter.get())

glean-core/android/src/test/java/mozilla/telemetry/glean/GleanFromJavaTest.java

@@ -6,7 +6,6 @@
 
 import android.content.Context;
 
-import androidx.test.core.app.ApplicationProvider;
 import androidx.work.testing.WorkManagerTestInitHelper;
 
 import org.junit.Before;
@@ -25,7 +24,7 @@ public class GleanFromJavaTest {
     // callable from Java. If something goes wrong, it should complain about missing
     // methods at build-time.
 
-    private Context appContext = ApplicationProvider.getApplicationContext();
+    private Context appContext = TestUtilKt.getContextWithMockedInfo("java-test");
 
     @Before
     public void setup() {

glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt

@@ -13,6 +13,7 @@ import androidx.test.core.app.ApplicationProvider
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import androidx.work.testing.WorkManagerTestInitHelper
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
 import org.json.JSONObject
@@ -197,7 +198,7 @@ internal fun waitForEnqueuedWorker(
             if (getWorkerStatus(context, workTag).isEnqueued) {
                 return@withTimeout
             }
-        } while (true)
+        } while (isActive)
     }
 }