fix(Android): drop zero frames in release mode (#607)

* fix(Android): drop zero frames in release mode

* use more main-safe suspend functions

* add note in PR template

* remove one last extra dispatcher arg

* don't bother with thread annotations

.github/pull_request_template.md

@@ -7,8 +7,10 @@ What is this PR for?
 iOS
 - [ ] If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?
   - [ ] Add temporary machine translations, marked "Needs Review"
+
 android
 - [ ] All user-facing strings added to strings resource
+- [ ] Expensive calculations are run in `withContext(Dispatchers.Default)` where possible
 
 ### Testing
 

androidApp/src/main/java/com/mbta/tid/mbta_app/android/map/HomeMapView.kt

@@ -12,6 +12,7 @@ import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -63,6 +64,7 @@ import com.mbta.tid.mbta_app.model.response.StopMapResponse
 import io.github.dellisd.spatialk.geojson.Position
 import kotlin.time.Duration.Companion.seconds
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.launch
 
 @OptIn(MapboxExperimental::class, ExperimentalPermissionsApi::class)
 @Composable
@@ -82,6 +84,7 @@ fun HomeMapView(
     var nearbyTransitSelectingLocation by nearbyTransitSelectingLocationState
     val previousNavEntry: NavBackStackEntry? = rememberPrevious(current = currentNavEntry)
 
+    val coroutineScope = rememberCoroutineScope()
     val layerManager = remember { LazyObjectQueue<MapLayerManager>() }
     var selectedStop by remember { mutableStateOf<Stop?>(null) }
 
@@ -128,7 +131,7 @@ fun HomeMapView(
         viewportProvider.animateTo(stop.position.toMapbox())
     }
 
-    fun updateDisplayedRoutesBasedOnStop() {
+    suspend fun updateDisplayedRoutesBasedOnStop() {
         val globalResponse = globalResponse ?: return
         val railRouteShapes = railRouteShapes ?: return
         val stopMapData = stopMapData ?: return
@@ -159,19 +162,19 @@ fun HomeMapView(
         }
     }
 
-    fun refreshRouteLineSource() {
+    suspend fun refreshRouteLineSource() {
         val routeData = railRouteLineData ?: return
         layerManager.run {
             updateRouteSourceData(RouteFeaturesBuilder.buildCollection(routeData).toMapbox())
         }
     }
 
-    fun refreshStopSource() {
+    suspend fun refreshStopSource() {
         val sourceData = stopSourceData ?: return
         layerManager.run { updateStopSourceData(sourceData) }
     }
 
-    fun handleNearbyNavRestoration() {
+    suspend fun handleNearbyNavRestoration() {
         if (
             previousNavEntry?.destination?.route?.contains("NearbyTransit") == true &&
                 currentNavEntry?.destination?.route?.contains("StopDetails") == true
@@ -186,7 +189,7 @@ fun HomeMapView(
         }
     }
 
-    fun handleNavChange() {
+    suspend fun handleNavChange() {
         handleNearbyNavRestoration()
         val stopId = currentNavEntry?.arguments?.getString("stopId")
         if (stopId == null) {
@@ -211,8 +214,10 @@ fun HomeMapView(
             mapEvents =
                 MapEvents(
                     onStyleLoaded = {
-                        layerManager.run {
-                            addLayers(if (isDarkMode) ColorPalette.dark else ColorPalette.light)
+                        coroutineScope.launch {
+                            layerManager.run {
+                                addLayers(if (isDarkMode) ColorPalette.dark else ColorPalette.light)
+                            }
                         }
                     },
                     onCameraChanged = { viewportProvider.updateCameraState(it.cameraState) }
@@ -299,8 +304,9 @@ fun HomeMapView(
             }
 
             MapEffect { map ->
-                if (layerManager.`object` == null)
-                    layerManager.`object` = MapLayerManager(map.mapboxMap, context)
+                if (layerManager.`object` == null) {
+                    layerManager.setObject(MapLayerManager(map.mapboxMap, context))
+                }
             }
 
             if (

androidApp/src/main/java/com/mbta/tid/mbta_app/android/map/MapLayerManager.kt

@@ -18,6 +18,8 @@ import com.mbta.tid.mbta_app.map.RouteLayerGenerator
 import com.mbta.tid.mbta_app.map.StopFeaturesBuilder
 import com.mbta.tid.mbta_app.map.StopIcons
 import com.mbta.tid.mbta_app.map.StopLayerGenerator
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 class MapLayerManager(val map: MapboxMap, context: Context) {
     init {
@@ -28,23 +30,27 @@ class MapLayerManager(val map: MapboxMap, context: Context) {
         }
     }
 
-    fun addSource(source: GeoJsonSource) {
-        this.map.addSource(source)
+    suspend fun addSource(source: GeoJsonSource) {
+        withContext(Dispatchers.Main) { map.addSource(source) }
     }
 
-    fun addLayers(colorPalette: ColorPalette) {
+    suspend fun addLayers(colorPalette: ColorPalette) {
         val layers: List<MapboxLayer> =
-            RouteLayerGenerator.createAllRouteLayers(colorPalette).map { it.toMapbox() } +
-                StopLayerGenerator.createStopLayers(colorPalette).map { it.toMapbox() }
-        for (layer in layers) {
-            if (map.styleLayerExists(checkNotNull(layer.layerId))) {
-                // Skip attempting to add layer if it already exists
-                continue
+            withContext(Dispatchers.Default) {
+                RouteLayerGenerator.createAllRouteLayers(colorPalette).map { it.toMapbox() } +
+                    StopLayerGenerator.createStopLayers(colorPalette).map { it.toMapbox() }
             }
-            if (map.styleLayerExists("puck")) {
-                map.addLayerBelow(layer, below = "puck")
-            } else {
-                map.addLayer(layer)
+        withContext(Dispatchers.Main) {
+            for (layer in layers) {
+                if (map.styleLayerExists(checkNotNull(layer.layerId))) {
+                    // Skip attempting to add layer if it already exists
+                    continue
+                }
+                if (map.styleLayerExists("puck")) {
+                    map.addLayerBelow(layer, below = "puck")
+                } else {
+                    map.addLayer(layer)
+                }
             }
         }
     }
@@ -55,24 +61,27 @@ class MapLayerManager(val map: MapboxMap, context: Context) {
         }
     }
 
-    private fun updateSourceData(sourceId: String, data: FeatureCollection) {
-        if (map.styleSourceExists(sourceId)) {
-            map.setStyleGeoJSONSourceData(
-                sourceId,
-                "",
-                GeoJSONSourceData(checkNotNull(data.features()))
-            )
+    private suspend fun updateSourceData(sourceId: String, data: FeatureCollection) {
+        // styleSourceExists is not thread safe, but setStyleGeoJSONSourceData is
+        if (withContext(Dispatchers.Main) { map.styleSourceExists(sourceId) }) {
+            withContext(Dispatchers.Default) {
+                map.setStyleGeoJSONSourceData(
+                    sourceId,
+                    "",
+                    GeoJSONSourceData(checkNotNull(data.features()))
+                )
+            }
         } else {
             val source = GeoJsonSource.Builder(sourceId).featureCollection(data).build()
             addSource(source)
         }
     }
 
-    fun updateRouteSourceData(routeData: FeatureCollection) {
+    suspend fun updateRouteSourceData(routeData: FeatureCollection) {
         updateSourceData(RouteFeaturesBuilder.routeSourceId, routeData)
     }
 
-    fun updateStopSourceData(stopData: FeatureCollection) {
+    suspend fun updateStopSourceData(stopData: FeatureCollection) {
         updateSourceData(StopFeaturesBuilder.stopSourceId, stopData)
     }
 }

androidApp/src/main/java/com/mbta/tid/mbta_app/android/map/MapViewModel.kt

@@ -1,18 +1,17 @@
 package com.mbta.tid.mbta_app.android.map
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.viewModelScope
 import com.mapbox.common.HttpServiceFactory
 import com.mapbox.common.MapboxOptions
 import com.mapbox.geojson.FeatureCollection
+import com.mbta.tid.mbta_app.android.util.rememberSuspend
 import com.mbta.tid.mbta_app.dependencyInjection.UsecaseDI
 import com.mbta.tid.mbta_app.map.RouteFeaturesBuilder
 import com.mbta.tid.mbta_app.map.RouteLineData
@@ -35,6 +34,7 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import kotlinx.datetime.Clock
 import kotlinx.datetime.Instant
 import org.koin.core.component.KoinComponent
@@ -103,18 +103,16 @@ open class MapViewModel(
     }
 
     override suspend fun globalMapData(now: Instant): GlobalMapData? =
-        globalResponse.first()?.let {
-            GlobalMapData(it, GlobalMapData.getAlertsByStop(it, alertsData, now))
+        withContext(Dispatchers.Default) {
+            globalResponse.first()?.let {
+                GlobalMapData(it, GlobalMapData.getAlertsByStop(it, alertsData, now))
+            }
         }
 
     @Composable
     override fun rememberGlobalMapData(now: Instant): GlobalMapData? {
-        var result by remember { mutableStateOf<GlobalMapData?>(null) }
         val globalResponse by this.globalResponse.collectAsState(initial = null)
-        LaunchedEffect(this.alertsData, globalResponse, now) {
-            launch(Dispatchers.IO) { result = globalMapData(now) }
-        }
-        return result
+        return rememberSuspend(this.alertsData, globalResponse, now) { globalMapData(now) }
     }
 
     override suspend fun refreshRouteLineData(now: Instant) {

androidApp/src/main/java/com/mbta/tid/mbta_app/android/nearbyTransit/NearbyTransitView.kt

@@ -22,6 +22,7 @@ import com.mbta.tid.mbta_app.android.state.getNearby
 import com.mbta.tid.mbta_app.android.state.getSchedule
 import com.mbta.tid.mbta_app.android.state.subscribeToPredictions
 import com.mbta.tid.mbta_app.android.util.managePinnedRoutes
+import com.mbta.tid.mbta_app.android.util.rememberSuspend
 import com.mbta.tid.mbta_app.android.util.timer
 import com.mbta.tid.mbta_app.model.Coordinate
 import com.mbta.tid.mbta_app.model.NearbyStaticData
@@ -32,6 +33,8 @@ import com.mbta.tid.mbta_app.model.response.GlobalResponse
 import com.mbta.tid.mbta_app.model.withRealtimeInfo
 import io.github.dellisd.spatialk.geojson.Position
 import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 @Composable
 fun NearbyTransitView(
@@ -58,7 +61,7 @@ fun NearbyTransitView(
     val (pinnedRoutes, togglePinnedRoute) = managePinnedRoutes()
 
     val nearbyWithRealtimeInfo =
-        remember(
+        rememberSuspend(
             nearby,
             globalResponse,
             targetLocation,
@@ -68,19 +71,21 @@ fun NearbyTransitView(
             now,
             pinnedRoutes
         ) {
-            if (targetLocation != null) {
-                nearby?.withRealtimeInfo(
-                    globalData = globalResponse,
-                    sortByDistanceFrom = targetLocation,
-                    schedules,
-                    predictions,
-                    alertData,
-                    now,
-                    pinnedRoutes.orEmpty(),
-                    useTripHeadsigns = false,
-                )
-            } else {
-                null
+            withContext(Dispatchers.Default) {
+                if (targetLocation != null) {
+                    nearby?.withRealtimeInfo(
+                        globalData = globalResponse,
+                        sortByDistanceFrom = targetLocation,
+                        schedules,
+                        predictions,
+                        alertData,
+                        now,
+                        pinnedRoutes.orEmpty(),
+                        useTripHeadsigns = false,
+                    )
+                } else {
+                    null
+                }
             }
         }
 

androidApp/src/main/java/com/mbta/tid/mbta_app/android/pages/StopDetailsPage.kt

@@ -3,20 +3,22 @@ package com.mbta.tid.mbta_app.android.pages
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.remember
 import androidx.compose.ui.Modifier
 import com.mapbox.maps.MapboxExperimental
 import com.mbta.tid.mbta_app.android.state.getGlobalData
 import com.mbta.tid.mbta_app.android.state.getSchedule
 import com.mbta.tid.mbta_app.android.state.subscribeToPredictions
 import com.mbta.tid.mbta_app.android.stopDetails.StopDetailsView
 import com.mbta.tid.mbta_app.android.util.managePinnedRoutes
+import com.mbta.tid.mbta_app.android.util.rememberSuspend
 import com.mbta.tid.mbta_app.android.util.timer
 import com.mbta.tid.mbta_app.model.Stop
 import com.mbta.tid.mbta_app.model.StopDetailsDepartures
 import com.mbta.tid.mbta_app.model.StopDetailsFilter
 import com.mbta.tid.mbta_app.model.response.AlertsStreamDataResponse
 import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 @Composable
 @ExperimentalMaterial3Api
@@ -41,7 +43,7 @@ fun StopDetailsPage(
     val (pinnedRoutes, togglePinnedRoute) = managePinnedRoutes()
 
     val departures =
-        remember(
+        rememberSuspend(
             stop,
             globalResponse,
             schedulesResponse,
@@ -50,18 +52,20 @@ fun StopDetailsPage(
             pinnedRoutes,
             now
         ) {
-            if (globalResponse != null) {
-                StopDetailsDepartures.fromData(
-                    stop,
-                    globalResponse,
-                    schedulesResponse,
-                    predictionsResponse,
-                    alertData,
-                    pinnedRoutes.orEmpty(),
-                    now,
-                    useTripHeadsigns = false,
-                )
-            } else null
+            withContext(Dispatchers.Default) {
+                if (globalResponse != null) {
+                    StopDetailsDepartures.fromData(
+                        stop,
+                        globalResponse,
+                        schedulesResponse,
+                        predictionsResponse,
+                        alertData,
+                        pinnedRoutes.orEmpty(),
+                        now,
+                        useTripHeadsigns = false,
+                    )
+                } else null
+            }
         }
 
     LaunchedEffect(departures) { updateDepartures(departures) }

androidApp/src/main/java/com/mbta/tid/mbta_app/android/util/LazyObjectQueue.kt

@@ -6,20 +6,22 @@ package com.mbta.tid.mbta_app.android.util
  */
 class LazyObjectQueue<T> {
     var `object`: T? = null
-        set(value) {
-            field = value
-            if (value != null) {
-                val oldQueue = queue
-                queue = mutableListOf()
-                for (op in oldQueue) {
-                    value.op()
-                }
+        private set
+
+    suspend fun setObject(value: T?) {
+        `object` = value
+        if (value != null) {
+            val oldQueue = queue
+            queue = mutableListOf()
+            for (op in oldQueue) {
+                value.op()
             }
         }
+    }
 
-    private var queue: MutableList<T.() -> Unit> = mutableListOf()
+    private var queue: MutableList<suspend T.() -> Unit> = mutableListOf()
 
-    fun run(op: T.() -> Unit) {
+    suspend fun run(op: suspend T.() -> Unit) {
         val `object` = this.`object`
         if (`object` != null) {
             `object`.op()