Use a different thread for each TreehouseApp (#2183)

* Use a different thread for each TreehouseApp

This should prevent different apps from competing with each
other for CPU resources, especially during the first few
seconds of the host app's launch, when there's a bunch of
CPU-bound work to do.

* Fix Android tests

CHANGELOG.md

@@ -17,6 +17,9 @@ Fixed:
 - Don't crash in `LazyList` when a scroll and content change occur in the same update.
 - Updating a flex container's margin now works correctly for Yoga-based layouts.
 
+Breaking:
+- The `TreehouseApp.Factory.dispatchers` property is removed, and callers should migrate to `TreehouseApp.dispatchers`. With this update each `TreehouseApp` has its own private thread so a shared `dispatchers` property no longer fits our implementation.
+
 Upgraded:
 - Zipline 1.15.0
 

redwood-treehouse-host/api/android/redwood-treehouse-host.api

@@ -95,7 +95,6 @@ public abstract class app/cash/redwood/treehouse/TreehouseApp : java/lang/AutoCl
 public abstract interface class app/cash/redwood/treehouse/TreehouseApp$Factory : java/lang/AutoCloseable {
 	public abstract fun create (Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;)Lapp/cash/redwood/treehouse/TreehouseApp;
 	public static synthetic fun create$default (Lapp/cash/redwood/treehouse/TreehouseApp$Factory;Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;ILjava/lang/Object;)Lapp/cash/redwood/treehouse/TreehouseApp;
-	public abstract fun getDispatchers ()Lapp/cash/redwood/treehouse/TreehouseDispatchers;
 }
 
 public abstract class app/cash/redwood/treehouse/TreehouseApp$Spec {

redwood-treehouse-host/api/jvm/redwood-treehouse-host.api

@@ -95,7 +95,6 @@ public abstract class app/cash/redwood/treehouse/TreehouseApp : java/lang/AutoCl
 public abstract interface class app/cash/redwood/treehouse/TreehouseApp$Factory : java/lang/AutoCloseable {
 	public abstract fun create (Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;)Lapp/cash/redwood/treehouse/TreehouseApp;
 	public static synthetic fun create$default (Lapp/cash/redwood/treehouse/TreehouseApp$Factory;Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;ILjava/lang/Object;)Lapp/cash/redwood/treehouse/TreehouseApp;
-	public abstract fun getDispatchers ()Lapp/cash/redwood/treehouse/TreehouseDispatchers;
 }
 
 public abstract class app/cash/redwood/treehouse/TreehouseApp$Spec {

redwood-treehouse-host/api/redwood-treehouse-host.klib.api

@@ -78,9 +78,6 @@ abstract class <#A: app.cash.redwood.treehouse/AppService> app.cash.redwood.tree
     abstract fun stop() // app.cash.redwood.treehouse/TreehouseApp.stop|stop(){}[0]
 
     abstract interface Factory : kotlin/AutoCloseable { // app.cash.redwood.treehouse/TreehouseApp.Factory|null[0]
-        abstract val dispatchers // app.cash.redwood.treehouse/TreehouseApp.Factory.dispatchers|{}dispatchers[0]
-            abstract fun <get-dispatchers>(): app.cash.redwood.treehouse/TreehouseDispatchers // app.cash.redwood.treehouse/TreehouseApp.Factory.dispatchers.<get-dispatchers>|<get-dispatchers>(){}[0]
-
         abstract fun <#A2: app.cash.redwood.treehouse/AppService> create(kotlinx.coroutines/CoroutineScope, app.cash.redwood.treehouse/TreehouseApp.Spec<#A2>, app.cash.redwood.treehouse/EventListener.Factory = ...): app.cash.redwood.treehouse/TreehouseApp<#A2> // app.cash.redwood.treehouse/TreehouseApp.Factory.create|create(kotlinx.coroutines.CoroutineScope;app.cash.redwood.treehouse.TreehouseApp.Spec<0:0>;app.cash.redwood.treehouse.EventListener.Factory){0§<app.cash.redwood.treehouse.AppService>}[0]
     }
 

redwood-treehouse-host/src/androidInstrumentedTest/kotlin/app/cash/redwood/treehouse/AndroidTreehouseDispatchersTest.kt

@@ -15,6 +15,7 @@
  */
 package app.cash.redwood.treehouse
 
-class AndroidTreehouseDispatchersTest : AbstractTreehouseDispatchersTest() {
-  override val treehouseDispatchers: TreehouseDispatchers = AndroidTreehouseDispatchers()
+internal class AndroidTreehouseDispatchersTest : AbstractTreehouseDispatchersTest() {
+  override fun newTreehouseDispatchers(applicationName: String) =
+    AndroidTreehouseDispatchers(applicationName)
 }

redwood-treehouse-host/src/androidMain/kotlin/app/cash/redwood/treehouse/AndroidTreehouseDispatchers.kt

@@ -17,20 +17,22 @@ package app.cash.redwood.treehouse
 
 import android.os.Looper
 import java.util.concurrent.Executors
+import kotlinx.coroutines.CloseableCoroutineDispatcher
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.asCoroutineDispatcher
 
 /**
  * Implements [TreehouseDispatchers] suitable for production Android use. This creates a background
  * thread for all Zipline work.
  */
-internal class AndroidTreehouseDispatchers : TreehouseDispatchers {
+internal class AndroidTreehouseDispatchers(applicationName: String) : TreehouseDispatchers {
   private var ziplineThread: Thread? = null
 
   /** The single thread that runs all JavaScript. We only have one QuickJS instance at a time. */
   private val executorService = Executors.newSingleThreadExecutor { runnable ->
-    Thread(null, runnable, "Treehouse", ZIPLINE_THREAD_STACK_SIZE.toLong())
+    Thread(null, runnable, "Treehouse $applicationName", ZIPLINE_THREAD_STACK_SIZE.toLong())
       .also { ziplineThread = it }
   }
 
@@ -49,3 +51,11 @@ internal class AndroidTreehouseDispatchers : TreehouseDispatchers {
     executorService.shutdown()
   }
 }
+
+@OptIn(ExperimentalCoroutinesApi::class) // CloseableCoroutineDispatcher is experimental.
+internal fun ziplineLoaderDispatcher(): CloseableCoroutineDispatcher {
+  val executorService = Executors.newSingleThreadExecutor { runnable ->
+    Thread(null, runnable, "ZiplineLoader")
+  }
+  return executorService.asCoroutineDispatcher()
+}

redwood-treehouse-host/src/androidMain/kotlin/app/cash/redwood/treehouse/AndroidTreehousePlatform.kt

@@ -45,4 +45,7 @@ internal class AndroidTreehousePlatform(
     maxSizeInBytes = maxSizeInBytes,
     loaderEventListener = loaderEventListener,
   )
+
+  override fun newDispatchers(applicationName: String): TreehouseDispatchers =
+    AndroidTreehouseDispatchers(applicationName)
 }

redwood-treehouse-host/src/androidMain/kotlin/app/cash/redwood/treehouse/treehouseAppFactoryAndroid.kt

@@ -19,11 +19,13 @@ import android.content.Context
 import app.cash.zipline.loader.LoaderEventListener
 import app.cash.zipline.loader.ManifestVerifier
 import app.cash.zipline.loader.asZiplineHttpClient
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import okhttp3.OkHttpClient
 import okio.FileSystem
 import okio.Path
 
 @Suppress("FunctionName")
+@OptIn(ExperimentalCoroutinesApi::class) // CloseableCoroutineDispatcher is experimental.
 public fun TreehouseAppFactory(
   context: Context,
   httpClient: OkHttpClient,
@@ -37,14 +39,14 @@ public fun TreehouseAppFactory(
   stateStore: StateStore = MemoryStateStore(),
 ): TreehouseApp.Factory = RealTreehouseApp.Factory(
   platform = AndroidTreehousePlatform(context),
-  dispatchers = AndroidTreehouseDispatchers(),
   httpClient = httpClient.asZiplineHttpClient(),
   frameClockFactory = AndroidChoreographerFrameClock.Factory(),
   manifestVerifier = manifestVerifier,
   embeddedFileSystem = embeddedFileSystem,
   embeddedDir = embeddedDir,
   cacheName = cacheName,
   cacheMaxSizeInBytes = cacheMaxSizeInBytes,
+  ziplineLoaderDispatcher = ziplineLoaderDispatcher(),
   loaderEventListener = loaderEventListener,
   concurrentDownloads = concurrentDownloads,
   stateStore = stateStore,

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/FakeZiplineLoaderDispatcher.kt

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2024 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood.treehouse
+
+import java.util.concurrent.Executor
+import kotlin.coroutines.CoroutineContext
+import kotlinx.coroutines.CloseableCoroutineDispatcher
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.Runnable
+import kotlinx.coroutines.test.TestScope
+
+@OptIn(ExperimentalCoroutinesApi::class) // CloseableCoroutineDispatcher is experimental.
+internal class FakeZiplineLoaderDispatcher(
+  testScope: TestScope,
+) : CloseableCoroutineDispatcher() {
+  private val delegate = testScope.dispatcher()
+
+  var closed = false
+    private set
+
+  override fun dispatch(context: CoroutineContext, block: Runnable) {
+    delegate.dispatch(context, block)
+  }
+
+  override val executor: Executor
+    get() = error("unexpected call")
+
+  override fun close() {
+    closed = true
+  }
+}

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt

@@ -189,20 +189,21 @@ class LeaksTest {
   }
 
   @Test
-  fun dispatchersNotClosedByApp() = runTest {
-    val dispatchers = FakeDispatchers(this)
-    val app = TreehouseTester(this, dispatchers).loadApp()
+  fun treehouseDispatchersClosedByApp() = runTest {
+    val treehouseTester = TreehouseTester(this)
+    val app = treehouseTester.loadApp()
+    assertThat(treehouseTester.openTreehouseDispatchersCount).isEqualTo(1)
     app.close()
-    assertThat(dispatchers.isClosed).isFalse()
+    assertThat(treehouseTester.openTreehouseDispatchersCount).isEqualTo(0)
+    assertThat(treehouseTester.ziplineLoaderDispatcher.closed).isFalse()
   }
 
   @Test
-  fun dispatchersNotLeakedByAppFactory() = runTest {
-    val dispatchers = FakeDispatchers(this)
-    val factory = TreehouseTester(this, dispatchers).treehouseAppFactory
-    assertThat(dispatchers.isClosed).isFalse()
-    factory.close()
-    assertThat(dispatchers.isClosed).isTrue()
+  fun ziplineLoaderDispatcherClosedByAppFactory() = runTest {
+    val treehouseTester = TreehouseTester(this)
+    assertThat(treehouseTester.ziplineLoaderDispatcher.closed).isFalse()
+    treehouseTester.treehouseAppFactory.close()
+    assertThat(treehouseTester.ziplineLoaderDispatcher.closed).isTrue()
   }
 
   /**

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/TreehouseTester.kt

@@ -22,6 +22,7 @@ import app.cash.zipline.loader.ZiplineHttpClient
 import com.example.redwood.testapp.treehouse.HostApi
 import com.example.redwood.testapp.treehouse.TestAppPresenter
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.first
@@ -40,7 +41,6 @@ import okio.Path.Companion.toPath
  */
 internal class TreehouseTester(
   private val testScope: TestScope,
-  dispatchers: FakeDispatchers = FakeDispatchers(testScope),
 ) {
   val eventLog = EventLog()
 
@@ -52,6 +52,8 @@ internal class TreehouseTester(
 
   private val kotlinZiplineDir = "../test-app/presenter-treehouse/build/zipline/Development".toPath()
 
+  private val returnedTreehouseDispatchers = mutableListOf<FakeDispatchers>()
+
   private val httpClient = object : ZiplineHttpClient() {
     override suspend fun download(
       url: String,
@@ -72,6 +74,13 @@ internal class TreehouseTester(
       maxSizeInBytes: Long,
       loaderEventListener: LoaderEventListener,
     ) = error("unexpected call")
+
+    override fun newDispatchers(
+      applicationName: String,
+    ): FakeDispatchers {
+      return FakeDispatchers(testScope)
+        .also { returnedTreehouseDispatchers += it }
+    }
   }
 
   private var appLifecycleAwaitingAFrame = MutableStateFlow<AppLifecycle?>(null)
@@ -86,21 +95,27 @@ internal class TreehouseTester(
     override fun create(scope: CoroutineScope, dispatchers: TreehouseDispatchers) = frameClock
   }
 
+  val ziplineLoaderDispatcher = FakeZiplineLoaderDispatcher(testScope)
+
+  @OptIn(ExperimentalCoroutinesApi::class) // CloseableCoroutineDispatcher is experimental.
   val treehouseAppFactory = RealTreehouseApp.Factory(
     platform = platform,
-    dispatchers = dispatchers,
     httpClient = httpClient,
     frameClockFactory = frameClockFactory,
     manifestVerifier = ManifestVerifier.NO_SIGNATURE_CHECKS,
     embeddedFileSystem = null,
     embeddedDir = null,
     cacheName = "cache",
     cacheMaxSizeInBytes = 0L,
+    ziplineLoaderDispatcher = ziplineLoaderDispatcher,
     concurrentDownloads = 1,
     loaderEventListener = LoaderEventListener.None,
     stateStore = MemoryStateStore(),
   )
 
+  val openTreehouseDispatchersCount: Int
+    get() = returnedTreehouseDispatchers.count { !it.isClosed }
+
   private val appSpec = object : TreehouseApp.Spec<TestAppPresenter>() {
     override val name: String
       get() = "test_app"

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/RealTreehouseApp.kt

@@ -22,7 +22,9 @@ import app.cash.zipline.loader.LoaderEventListener
 import app.cash.zipline.loader.ManifestVerifier
 import app.cash.zipline.loader.ZiplineHttpClient
 import app.cash.zipline.loader.ZiplineLoader
+import kotlinx.coroutines.CloseableCoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.mapNotNull
@@ -33,6 +35,7 @@ internal class RealTreehouseApp<A : AppService> private constructor(
   private val factory: Factory,
   private val appScope: CoroutineScope,
   override val spec: Spec<A>,
+  override val dispatchers: TreehouseDispatchers,
   eventListenerFactory: EventListener.Factory,
 ) : TreehouseApp<A>() {
   /** This property is confined to [TreehouseDispatchers.ui]. */
@@ -41,8 +44,6 @@ internal class RealTreehouseApp<A : AppService> private constructor(
   /** Non-null until this app is closed. This property is confined to [TreehouseDispatchers.ui]. */
   private var eventListenerFactory: EventListener.Factory? = eventListenerFactory
 
-  override val dispatchers = factory.dispatchers
-
   private val codeHost = object : CodeHost<A>(
     dispatchers = dispatchers,
     appScope = appScope,
@@ -122,6 +123,8 @@ internal class RealTreehouseApp<A : AppService> private constructor(
     if (!spec.loadCodeFromNetworkOnly) {
       loader = loader.withCache(
         cache = factory.cache.value,
+        // TODO(jwilson): use this once we update Zipline.
+        // cacheDispatcher = factory.ziplineLoaderDispatcher,
       )
 
       if (factory.embeddedFileSystem != null && factory.embeddedDir != null) {
@@ -165,18 +168,25 @@ internal class RealTreehouseApp<A : AppService> private constructor(
     closed = true
     eventListenerFactory = null
     stop()
+    dispatchers.close()
   }
 
+  /**
+   * @param ziplineLoaderDispatcher a dispatcher backed by a single thread, that's owned by this
+   *   factory. If it's a [CloseableCoroutineDispatcher], it will be closed when this factory is
+   *   closed.
+   */
+  @OptIn(ExperimentalCoroutinesApi::class) // CloseableCoroutineDispatcher is experimental.
   class Factory internal constructor(
     private val platform: TreehousePlatform,
-    override val dispatchers: TreehouseDispatchers,
     internal val httpClient: ZiplineHttpClient,
     internal val frameClockFactory: FrameClock.Factory,
     internal val manifestVerifier: ManifestVerifier,
     internal val embeddedFileSystem: FileSystem?,
     internal val embeddedDir: Path?,
     private val cacheName: String,
     private val cacheMaxSizeInBytes: Long,
+    internal val ziplineLoaderDispatcher: CloseableCoroutineDispatcher,
     private val loaderEventListener: LoaderEventListener,
     internal val concurrentDownloads: Int,
     internal val stateStore: StateStore,
@@ -194,14 +204,20 @@ internal class RealTreehouseApp<A : AppService> private constructor(
       appScope: CoroutineScope,
       spec: Spec<A>,
       eventListenerFactory: EventListener.Factory,
-    ): TreehouseApp<A> = RealTreehouseApp(this, appScope, spec, eventListenerFactory)
+    ): TreehouseApp<A> = RealTreehouseApp(
+      factory = this,
+      appScope = appScope,
+      spec = spec,
+      dispatchers = platform.newDispatchers(spec.name),
+      eventListenerFactory = eventListenerFactory,
+    )
 
     override fun close() {
       if (cache.isInitialized()) {
         cache.value.close()
       }
 
-      dispatchers.close()
+      ziplineLoaderDispatcher.close()
     }
   }
 }

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseApp.kt

@@ -104,8 +104,6 @@ public abstract class TreehouseApp<A : AppService> : AutoCloseable {
    */
   @ObjCName("TreehouseAppFactory", exact = true)
   public interface Factory : AutoCloseable {
-    public val dispatchers: TreehouseDispatchers
-
     public fun <A : AppService> create(
       appScope: CoroutineScope,
       spec: Spec<A>,

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseDispatchers.kt

@@ -25,6 +25,9 @@ import kotlinx.coroutines.CoroutineDispatcher
  *  * [zipline] executes dispatched tasks on the thread where downloaded code executes.
  *
  * This class makes it easier to specify invariants on which dispatcher is expected for which work.
+ *
+ * Each [TreehouseApp] gets its own instance of this class and has its own private Zipline thread.
+ * All apps share the same UI thread.
  */
 @ObjCName("TreehouseDispatchers", exact = true)
 public interface TreehouseDispatchers : AutoCloseable {
@@ -48,10 +51,9 @@ public interface TreehouseDispatchers : AutoCloseable {
 
   /**
    * Release the threads owned by this instance. On most platforms this will not release the UI
-   * thread, as it is not owned by this instance.
+   * thread as it is not owned by this instance.
    *
-   * Most applications should not to call this; instead they should allow these dispatchers to
-   * run until the process exits. This may be useful in tests.
+   * This is called by [TreehouseApp.close].
    */
   public override fun close()
 }

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehousePlatform.kt

@@ -24,4 +24,8 @@ internal interface TreehousePlatform {
     maxSizeInBytes: Long,
     loaderEventListener: LoaderEventListener,
   ): ZiplineCache
+
+  fun newDispatchers(
+    applicationName: String,
+  ): TreehouseDispatchers
 }