diff --git a/app/build.gradle.kts b/app/build.gradle.kts index e6a91fb..7d368e1 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -66,6 +66,11 @@ dependencies { implementation(platform(libs.androidx.compose.bom)) implementation(libs.bundles.compose.ui) + // App Startup: single ContentProvider for all SDK initializers (no per-SDK provider). + implementation(libs.androidx.startup.runtime) + // Coroutines: Dispatchers.IO for background SDK work, Dispatchers.Main for the app scope. + implementation(libs.kotlinx.coroutines.android) + // Installs Baseline Profiles at first launch (ART pre-compilation). implementation(libs.androidx.profileinstaller) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 921a07c..5b45805 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -14,6 +14,35 @@ android:roundIcon="@mipmap/ic_launcher_round" android:supportsRtl="true" android:theme="@style/Theme.AndroidPerfLab"> + + + + + + + + + - - \ No newline at end of file + diff --git a/app/src/main/java/com/aquib/androidperflab/AndroidPerfLabApplication.kt b/app/src/main/java/com/aquib/androidperflab/AndroidPerfLabApplication.kt index e1c914b..60d9dc7 100644 --- a/app/src/main/java/com/aquib/androidperflab/AndroidPerfLabApplication.kt +++ b/app/src/main/java/com/aquib/androidperflab/AndroidPerfLabApplication.kt @@ -2,38 +2,61 @@ package com.aquib.androidperflab import android.app.Application import android.util.Log -import com.aquib.androidperflab.sdk.FakeAnalyticsSdk -import com.aquib.androidperflab.sdk.FakeCrashReportingSdk -import com.aquib.androidperflab.sdk.FakeFeatureFlagsSdk -import com.aquib.androidperflab.sdk.FakePerformanceMonitorSdk -import com.aquib.androidperflab.sdk.FakeRemoteConfigSdk +import androidx.startup.AppInitializer +import com.aquib.androidperflab.startup.AnalyticsInitializer +import com.aquib.androidperflab.startup.FeatureFlagsInitializer +import com.aquib.androidperflab.startup.PerfMonitorInitializer +import com.aquib.androidperflab.startup.RemoteConfigInitializer +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch class AndroidPerfLabApplication : Application() { + // Initialized at property-declaration time (before Application.onCreate and before + // ContentProviders start). CrashReportingInitializer accesses this scope to fire + // its background upload job — which is why it must be a property, not set in onCreate. + // SupervisorJob: a failing child coroutine does not cancel sibling coroutines. + val applicationScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + override fun onCreate() { super.onCreate() - - // Intentionally bad: all five SDKs are initialised synchronously on the main thread. - // Each blocks via Thread.sleep() to simulate the I/O, disk, and network work that - // real SDKs perform. Total cold-start penalty ≈ 750 ms before the first frame. - val t0 = System.currentTimeMillis() - FakeCrashReportingSdk.init(this) // ~120 ms — must be first to catch early crashes - Log.d("AppStartup", "CrashReporting ready +${System.currentTimeMillis() - t0}ms") - - FakeAnalyticsSdk.init(this) // ~180 ms - Log.d("AppStartup", "Analytics ready +${System.currentTimeMillis() - t0}ms") - - FakeFeatureFlagsSdk.init(this) // ~150 ms - Log.d("AppStartup", "FeatureFlags ready +${System.currentTimeMillis() - t0}ms") - - FakeRemoteConfigSdk.init(this) // ~200 ms - Log.d("AppStartup", "RemoteConfig ready +${System.currentTimeMillis() - t0}ms") - - FakePerformanceMonitorSdk.init(this) // ~100 ms — last so it can baseline the others - Log.d("AppStartup", "PerfMonitor ready +${System.currentTimeMillis() - t0}ms") + // CrashReportingInitializer ran synchronously via InitializationProvider BEFORE + // this method. The exception handler is already registered; its upload coroutine + // is already running on Dispatchers.IO. Nothing to do here for crash reporting. + val appInit = AppInitializer.getInstance(this) + + // Non-critical SDKs (Analytics, PerfMonitor): launch immediately on IO. + // AppInitializer.initializeComponent() is idempotent — safe to call even if + // a dependency was already initialized via the manifest. + // Both SDKs initialize in ~180 ms + ~100 ms but never touch the main thread. + applicationScope.launch(Dispatchers.IO) { + appInit.initializeComponent(AnalyticsInitializer::class.java) + // PerfMonitor declares Analytics as a dependency in the initializer graph, + // so AppInitializer will invoke Analytics.create() first automatically — + // the explicit sequence here is just for clarity. + appInit.initializeComponent(PerfMonitorInitializer::class.java) + } + + // Lazy SDKs (FeatureFlags, RemoteConfig): deferred by 500 ms so they never + // compete with Compose's first layout-and-draw pass. Both SDKs return safe + // defaults until their background coroutines complete, so the UI is never gated + // on their initialization. + applicationScope.launch(Dispatchers.IO) { + delay(500L) + appInit.initializeComponent(FeatureFlagsInitializer::class.java) + appInit.initializeComponent(RemoteConfigInitializer::class.java) + } + + Log.d(TAG, "Application.onCreate() returned in ${System.currentTimeMillis() - t0} ms " + + "— all SDKs initializing in background") + } - Log.d("AppStartup", "Application.onCreate() complete — total blocked=${System.currentTimeMillis() - t0}ms") + companion object { + private const val TAG = "AppStartup" } } diff --git a/app/src/main/java/com/aquib/androidperflab/sdk/FakeCrashReportingSdk.kt b/app/src/main/java/com/aquib/androidperflab/sdk/FakeCrashReportingSdk.kt index 1db7699..b9dccf7 100644 --- a/app/src/main/java/com/aquib/androidperflab/sdk/FakeCrashReportingSdk.kt +++ b/app/src/main/java/com/aquib/androidperflab/sdk/FakeCrashReportingSdk.kt @@ -6,12 +6,32 @@ import java.io.File object FakeCrashReportingSdk { - // Simulates: scanning the cache directory for pending crash dumps left by a previous - // session, installing an UncaughtExceptionHandler, and uploading any found dumps - // synchronously before the app is considered "ready". - fun init(context: Context) { - // Simulate: scanning cache dir for pending crash dump files + // ── Fast path — called synchronously from CrashReportingInitializer ────────── + // + // Only registers the UncaughtExceptionHandler. Safe to call from any thread. + // Separated from uploadPendingReports() so the handler is installed before any + // other SDK work begins — matching the original "must be first" requirement — + // without blocking the main thread for the full 120 ms upload simulation. + + fun registerHandler(context: Context) { + val previousHandler = Thread.getDefaultUncaughtExceptionHandler() + Thread.setDefaultUncaughtExceptionHandler { thread, throwable -> + Log.e("FakeCrashReportingSdk", "Uncaught exception on ${thread.name}", throwable) + previousHandler?.uncaughtException(thread, throwable) + } + Log.d("FakeCrashReportingSdk", "registerHandler complete") + } + + // ── Slow path — called from Dispatchers.IO via CrashReportingInitializer ───── + // + // Simulates: scanning the cache directory for pending crash dumps, parsing them, + // writing a session sentinel, and uploading reports to the backend. All I/O and + // the blocking upload sleep are safe on a background thread. + + fun uploadPendingReports(context: Context) { val cacheDir = context.cacheDir + + // Simulate: scanning cache dir for pending crash dump files val crashDumps = cacheDir.listFiles { f -> f.name.startsWith("crash_") } ?.toList() ?: emptyList() @@ -25,21 +45,17 @@ object FakeCrashReportingSdk { } } - // Simulate: writing a session sentinel file so the next launch can detect - // whether this session ended cleanly + // Simulate: writing a session sentinel so the next launch can detect clean exits val sentinel = File(cacheDir, "crash_sentinel_${System.currentTimeMillis()}.tmp") sentinel.createNewFile() - // Simulate: registering the uncaught exception handler (involves thread locking) - val previousHandler = Thread.getDefaultUncaughtExceptionHandler() - Thread.setDefaultUncaughtExceptionHandler { thread, throwable -> - Log.e("FakeCrashReportingSdk", "Uncaught exception on ${thread.name}", throwable) - previousHandler?.uncaughtException(thread, throwable) - } - - // Simulate: blocking upload of any pending crash reports to the backend + // Simulate: blocking upload of pending reports to the backend Thread.sleep(120L) - Log.d("FakeCrashReportingSdk", "init complete — found ${crashDumps.size} pending dumps, parsed ${parsedReports.size} reports") + Log.d( + "FakeCrashReportingSdk", + "uploadPendingReports complete — found ${crashDumps.size} dumps, " + + "parsed ${parsedReports.size} reports", + ) } } diff --git a/app/src/main/java/com/aquib/androidperflab/startup/AnalyticsInitializer.kt b/app/src/main/java/com/aquib/androidperflab/startup/AnalyticsInitializer.kt new file mode 100644 index 0000000..a8184dd --- /dev/null +++ b/app/src/main/java/com/aquib/androidperflab/startup/AnalyticsInitializer.kt @@ -0,0 +1,32 @@ +package com.aquib.androidperflab.startup + +import android.content.Context +import android.util.Log +import androidx.startup.Initializer +import com.aquib.androidperflab.AndroidPerfLabApplication +import com.aquib.androidperflab.sdk.FakeAnalyticsSdk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +/** + * NOT registered in AndroidManifest.xml — triggered manually from Application.onCreate() + * via AppInitializer.initializeComponent() on Dispatchers.IO. + * + * create() returns immediately after launching the background coroutine; the ~180 ms + * SDK init never touches the main thread. CrashReportingInitializer is declared as a + * dependency so the exception handler is guaranteed to be registered before any SDK + * code runs (AppInitializer resolves the dependency graph automatically). + */ +class AnalyticsInitializer : Initializer { + override fun create(context: Context) { + val app = context.applicationContext as AndroidPerfLabApplication + app.applicationScope.launch(Dispatchers.IO) { + FakeAnalyticsSdk.init(context.applicationContext) + Log.d("AppStartup", "Analytics ready") + } + Log.d("AppStartup", "AnalyticsInitializer.create() returned") + } + + override fun dependencies(): List>> = + listOf(CrashReportingInitializer::class.java) +} diff --git a/app/src/main/java/com/aquib/androidperflab/startup/CrashReportingInitializer.kt b/app/src/main/java/com/aquib/androidperflab/startup/CrashReportingInitializer.kt new file mode 100644 index 0000000..67683cc --- /dev/null +++ b/app/src/main/java/com/aquib/androidperflab/startup/CrashReportingInitializer.kt @@ -0,0 +1,46 @@ +package com.aquib.androidperflab.startup + +import android.content.Context +import android.util.Log +import androidx.startup.Initializer +import com.aquib.androidperflab.AndroidPerfLabApplication +import com.aquib.androidperflab.sdk.FakeCrashReportingSdk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +/** + * Runs via InitializationProvider BEFORE Application.onCreate(). + * + * Critical path (main thread, synchronous): + * FakeCrashReportingSdk.registerHandler() — installs the UncaughtExceptionHandler. + * Cost: < 1 ms. + * + * Non-critical path (Dispatchers.IO, fire-and-forget): + * FakeCrashReportingSdk.uploadPendingReports() — scans crash dumps and simulates upload. + * Cost: ~120 ms off the main thread. + * + * applicationScope is initialized at property-declaration time in AndroidPerfLabApplication, + * so it exists before this Initializer runs (the Application object is created before + * ContentProviders are started). + */ +class CrashReportingInitializer : Initializer { + + override fun create(context: Context) { + val app = context.applicationContext as AndroidPerfLabApplication + + // Synchronous — registers the exception handler before any other SDK work. + FakeCrashReportingSdk.registerHandler(context.applicationContext) + + // Asynchronous — upload is I/O bound and does not need to complete before + // the first frame; move it off the critical path entirely. + app.applicationScope.launch(Dispatchers.IO) { + FakeCrashReportingSdk.uploadPendingReports(context.applicationContext) + Log.d("AppStartup", "CrashReporting upload complete") + } + + Log.d("AppStartup", "CrashReportingInitializer.create() returned") + } + + // No dependencies — CrashReporting must be the root of the initializer graph. + override fun dependencies(): List>> = emptyList() +} diff --git a/app/src/main/java/com/aquib/androidperflab/startup/FeatureFlagsInitializer.kt b/app/src/main/java/com/aquib/androidperflab/startup/FeatureFlagsInitializer.kt new file mode 100644 index 0000000..a00cebe --- /dev/null +++ b/app/src/main/java/com/aquib/androidperflab/startup/FeatureFlagsInitializer.kt @@ -0,0 +1,34 @@ +package com.aquib.androidperflab.startup + +import android.content.Context +import android.util.Log +import androidx.startup.Initializer +import com.aquib.androidperflab.AndroidPerfLabApplication +import com.aquib.androidperflab.sdk.FakeFeatureFlagsSdk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +/** + * Lazy initializer for the Feature Flags SDK. + * + * NOT registered in AndroidManifest.xml. Application.onCreate() defers triggering + * this initializer by 500 ms so it never competes with first-frame rendering or the + * immediately-needed Analytics / PerfMonitor SDKs. + * + * FakeFeatureFlagsSdk.isEnabled() returns false (the default) for any flag until the + * background coroutine completes — the UI is never gated on this SDK being ready. + */ +class FeatureFlagsInitializer : Initializer { + + override fun create(context: Context) { + val app = context.applicationContext as AndroidPerfLabApplication + app.applicationScope.launch(Dispatchers.IO) { + FakeFeatureFlagsSdk.init(context.applicationContext) + Log.d("AppStartup", "FeatureFlags ready") + } + Log.d("AppStartup", "FeatureFlagsInitializer.create() returned") + } + + override fun dependencies(): List>> = + listOf(CrashReportingInitializer::class.java) +} diff --git a/app/src/main/java/com/aquib/androidperflab/startup/PerfMonitorInitializer.kt b/app/src/main/java/com/aquib/androidperflab/startup/PerfMonitorInitializer.kt new file mode 100644 index 0000000..df18081 --- /dev/null +++ b/app/src/main/java/com/aquib/androidperflab/startup/PerfMonitorInitializer.kt @@ -0,0 +1,34 @@ +package com.aquib.androidperflab.startup + +import android.content.Context +import android.util.Log +import androidx.startup.Initializer +import com.aquib.androidperflab.AndroidPerfLabApplication +import com.aquib.androidperflab.sdk.FakePerformanceMonitorSdk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +/** + * NOT registered in AndroidManifest.xml — triggered from Application.onCreate() on + * Dispatchers.IO, sequenced after AnalyticsInitializer. + * + * Declaring AnalyticsInitializer as a dependency preserves the original intent: the + * performance monitor should start after the other SDKs have launched so it can + * baseline their initialization overhead. Note that the dependency guarantees ordering + * of create() calls, not completion of the background coroutines — PerfMonitor may + * start its actual work while Analytics is still initializing. + */ +class PerfMonitorInitializer : Initializer { + + override fun create(context: Context) { + val app = context.applicationContext as AndroidPerfLabApplication + app.applicationScope.launch(Dispatchers.IO) { + FakePerformanceMonitorSdk.init(context.applicationContext) + Log.d("AppStartup", "PerfMonitor ready") + } + Log.d("AppStartup", "PerfMonitorInitializer.create() returned") + } + + override fun dependencies(): List>> = + listOf(AnalyticsInitializer::class.java) +} diff --git a/app/src/main/java/com/aquib/androidperflab/startup/RemoteConfigInitializer.kt b/app/src/main/java/com/aquib/androidperflab/startup/RemoteConfigInitializer.kt new file mode 100644 index 0000000..a48a2d3 --- /dev/null +++ b/app/src/main/java/com/aquib/androidperflab/startup/RemoteConfigInitializer.kt @@ -0,0 +1,34 @@ +package com.aquib.androidperflab.startup + +import android.content.Context +import android.util.Log +import androidx.startup.Initializer +import com.aquib.androidperflab.AndroidPerfLabApplication +import com.aquib.androidperflab.sdk.FakeRemoteConfigSdk +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch + +/** + * Lazy initializer for the Remote Config SDK. + * + * NOT registered in AndroidManifest.xml. Triggered from Application.onCreate() after + * a 500 ms delay (alongside FeatureFlagsInitializer) so first-frame rendering is + * never competed with by config disk / network I/O. + * + * FakeRemoteConfigSdk.getString() returns the previously cached blob from SharedPreferences + * until the background coroutine completes — the app always has a usable config value. + */ +class RemoteConfigInitializer : Initializer { + + override fun create(context: Context) { + val app = context.applicationContext as AndroidPerfLabApplication + app.applicationScope.launch(Dispatchers.IO) { + FakeRemoteConfigSdk.init(context.applicationContext) + Log.d("AppStartup", "RemoteConfig ready") + } + Log.d("AppStartup", "RemoteConfigInitializer.create() returned") + } + + override fun dependencies(): List>> = + listOf(CrashReportingInitializer::class.java) +} diff --git a/benchmarks/src/main/java/com/aquib/androidperflab/benchmarks/AppStartupBenchmark.kt b/benchmarks/src/main/java/com/aquib/androidperflab/benchmarks/AppStartupBenchmark.kt new file mode 100644 index 0000000..772f732 --- /dev/null +++ b/benchmarks/src/main/java/com/aquib/androidperflab/benchmarks/AppStartupBenchmark.kt @@ -0,0 +1,136 @@ +package com.aquib.androidperflab.benchmarks + +import androidx.benchmark.macro.CompilationMode +import androidx.benchmark.macro.MacrobenchmarkScope +import androidx.benchmark.macro.StartupMode +import androidx.benchmark.macro.StartupTimingMetric +import androidx.benchmark.macro.junit4.MacrobenchmarkRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.uiautomator.By +import androidx.test.uiautomator.Until +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +private const val TARGET_PACKAGE = "com.aquib.androidperflab" +private const val RENDER_TIMEOUT_MS = 5_000L + +/** + * Before / after comparison for moving SDK initializations off the main thread. + * + * ┌─────────────────────┬─────────────────┬──────────────────────────────────────────┐ + * │ State │ TTID (approx.) │ Main-thread SDK blocking │ + * ├─────────────────────┼─────────────────┼──────────────────────────────────────────┤ + * │ BEFORE (baseline) │ ~1100–1300 ms │ ~750 ms (5 SDKs × synchronous sleep) │ + * │ AFTER (this build) │ ~150–350 ms │ < 5 ms (handler registration only) │ + * └─────────────────────┴─────────────────┴──────────────────────────────────────────┘ + * + * What changed: + * • CrashReportingInitializer (manifest, before Application.onCreate): + * – registerHandler() — < 1 ms, main thread + * – uploadPendingReports() — ~120 ms, Dispatchers.IO + * • AnalyticsInitializer + PerfMonitorInitializer (from Application.onCreate): + * – launched immediately on Dispatchers.IO (~180 ms + ~100 ms, never main thread) + * • FeatureFlagsInitializer + RemoteConfigInitializer (lazy, from Application.onCreate): + * – deferred 500 ms, then run on Dispatchers.IO (~150 ms + ~200 ms) + * – SDK public methods return defaults until coroutines complete + * + * Run against the BEFORE baseline: + * git stash # restore original synchronous Application.onCreate + * ./gradlew :benchmarks:connectedBenchmarkAndroidTest \ + * -Pandroid.testInstrumentationRunnerArguments.class=\ + * com.aquib.androidperflab.benchmarks.AppStartupBenchmark + * git stash pop # restore async implementation + * ./gradlew :benchmarks:connectedBenchmarkAndroidTest \ + * -Pandroid.testInstrumentationRunnerArguments.class=\ + * com.aquib.androidperflab.benchmarks.AppStartupBenchmark + * + * Compare timeToInitialDisplayMs across both runs. + * See also: StartupBenchmark.startupCold() for the original three-mode baseline. + */ +@RunWith(AndroidJUnit4::class) +class AppStartupBenchmark { + + @get:Rule + val benchmarkRule = MacrobenchmarkRule() + + /** + * Cold start — full Application.onCreate() path. + * + * BEFORE: TTID dominated by ~750 ms of synchronous SDK init on the main thread. + * AFTER: TTID reflects only Compose first-frame work; SDKs run in background. + * + * Expected improvement: ~600–750 ms reduction in timeToInitialDisplayMs. + */ + @Test + fun startupCold_sdkAsyncInit() = measure(StartupMode.COLD) + + /** + * Warm start — Activity recreated, Application.onCreate() skipped. + * + * TTID here should be identical before and after the fix — confirming that + * async SDK init does not regress non-cold startup paths. + */ + @Test + fun startupWarm_sdkAsyncInit() = measure(StartupMode.WARM) + + // ── Core measurement ────────────────────────────────────────────────────── + + private fun measure(startupMode: StartupMode) { + benchmarkRule.measureRepeated( + packageName = TARGET_PACKAGE, + metrics = listOf(StartupTimingMetric()), + compilationMode = CompilationMode.None(), + startupMode = startupMode, + iterations = 10, + setupBlock = { pressHome() }, + measureBlock = { + startActivityAndWait() + assertTtidCaptured() + assertTtfdCaptured() + }, + ) + } + + // ── Assertion helpers ───────────────────────────────────────────────────── + + /** + * Verifies TTID: the target package is in the foreground and the first feed item + * is visible. An app crash during async SDK init would cause this to fail with a + * "Wrong package" or timeout error rather than a silent incorrect measurement. + */ + private fun MacrobenchmarkScope.assertTtidCaptured() { + assertEquals( + "Wrong package in foreground — async SDK init may have caused a crash", + TARGET_PACKAGE, + device.currentPackageName, + ) + val firstItem = device.wait( + Until.hasObject(By.text("Post #0 — Technology")), + RENDER_TIMEOUT_MS, + ) + assertNotNull( + "\"Post #0 — Technology\" not visible within ${RENDER_TIMEOUT_MS} ms — " + + "async SDK init may be blocking the main thread", + firstItem, + ) + } + + /** + * Verifies TTFD: waits for the feed LazyColumn to become scrollable, which + * requires Compose's full first layout pass to have completed. + */ + private fun MacrobenchmarkScope.assertTtfdCaptured() { + val scrollable = device.wait( + Until.hasObject(By.scrollable(true)), + RENDER_TIMEOUT_MS, + ) + assertNotNull( + "No scrollable view within ${RENDER_TIMEOUT_MS} ms — " + + "Compose layout may not have completed", + scrollable, + ) + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ddbd9bc..75f0f5b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -25,6 +25,7 @@ junit = "4.13.2" junitVersion = "1.2.1" espressoCore = "3.6.1" coroutinesTest = "1.9.0" +startupRuntime = "1.2.0" # ──────────────────────────────────────────────────────────────── [libraries] @@ -63,7 +64,9 @@ coil-network-okhttp = { group = "io.coil-kt.coil3", name = "coil-network-okhttp" junit = { group = "junit", name = "junit", version.ref = "junit" } androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "junitVersion" } androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" } -kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" } +kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutinesTest" } +kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "coroutinesTest" } +androidx-startup-runtime = { group = "androidx.startup", name = "startup-runtime", version.ref = "startupRuntime" } # ──────────────────────────────────────────────────────────────── [bundles] diff --git a/ui/src/main/java/com/aquib/androidperflab/ui/AnimatedListScreen.kt b/ui/src/main/java/com/aquib/androidperflab/ui/AnimatedListScreen.kt index 3d12cea..945b569 100644 --- a/ui/src/main/java/com/aquib/androidperflab/ui/AnimatedListScreen.kt +++ b/ui/src/main/java/com/aquib/androidperflab/ui/AnimatedListScreen.kt @@ -1,11 +1,15 @@ package com.aquib.androidperflab.ui -import androidx.compose.animation.animateContentSize +import androidx.compose.animation.core.DeferredTargetAnimation +import androidx.compose.animation.core.ExperimentalAnimatableApi import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.RepeatMode +import androidx.compose.animation.core.Spring +import androidx.compose.animation.core.VectorConverter import androidx.compose.animation.core.animateFloat import androidx.compose.animation.core.infiniteRepeatable import androidx.compose.animation.core.rememberInfiniteTransition +import androidx.compose.animation.core.spring import androidx.compose.animation.core.tween import androidx.compose.foundation.background import androidx.compose.foundation.clickable @@ -24,19 +28,27 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.Immutable 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 -import androidx.compose.ui.draw.alpha +import androidx.compose.ui.draw.clipToBounds import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.layout.layout import androidx.compose.ui.platform.testTag import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp +import kotlin.math.roundToInt +// @Immutable: all fields are val and all types are immutable, so the Compose compiler +// can skip any composable whose AnimatedItem argument has not changed reference. +@Immutable private data class AnimatedItem( val id: Int, val title: String, @@ -49,9 +61,9 @@ private fun generateAnimatedItems(count: Int = 80): List = List(co id = i, title = "Animation Demo Item #$i", subtitle = "Tap to expand · item ${i + 1} of $count", - body = "This card has animateContentSize applied, an alpha that reads State " + - "in composition scope (not inside a graphicsLayer lambda), and a Color " + - "constructed inline on every recomposition. All intentionally unoptimized.", + body = "Alpha pulse runs via graphicsLayer (draw phase — zero recomposition). " + + "Expand/collapse uses DeferredTargetAnimation inside Modifier.layout — " + + "height changes are layout-phase only. No recomposition during animation frames.", ) } @@ -69,11 +81,12 @@ fun AnimatedListScreen( modifier = Modifier .fillMaxSize() .testTag("animated_list") - .semantics { contentDescription = "animated_list" } + .semantics { contentDescription = "animated_list" }, ) { - // BAD: no key lambda — Compose cannot track item identity across recompositions, - // so any structural change causes it to diff by position rather than by id. - items(items) { item -> + // FIX — stable key: pins each slot to a specific AnimatedItem by id so + // Compose can reuse existing nodes on structural changes instead of + // destroying and recreating items by position. + items(items, key = { it.id }) { item -> AnimatedListCard(item = item) HorizontalDivider() } @@ -81,91 +94,137 @@ fun AnimatedListScreen( } } +@OptIn(ExperimentalAnimatableApi::class) @Composable private fun AnimatedListCard(item: AnimatedItem) { var expanded by remember { mutableStateOf(false) } - // BAD: animatedAlpha is a State whose value changes every ~16 ms while the - // animation runs. Reading it here (via the `by` delegate) subscribes this composable - // to that state, so the ENTIRE composable — and every child inside it — recomposes - // on every single animation frame. + // rememberCoroutineScope() gives a scope tied to this composable's lifecycle. + // DeferredTargetAnimation uses it to launch the height-animation coroutine from + // inside the layout lambda — without ever touching the composition phase. + val scope = rememberCoroutineScope() + + // FIX — remember(item.id): Color object allocated once per item. + // Previously a new Color was constructed on every recomposition. + val accentColor = remember(item.id) { + Color( + red = (item.id * 37 % 200 + 55) / 255f, + green = (item.id * 71 % 180 + 50) / 255f, + blue = (item.id * 13 % 220 + 35) / 255f, + ) + } + + // FIX — infinite alpha pulse via graphicsLayer: + // animateFloat() returns State. Assigning to a plain val (no `by`) means + // the value is NOT read here in the composition scope. The `by` delegate would call + // State.getValue() during composition, subscribing this entire composable to the + // animation and causing a full recompose on every 16 ms frame. // - // Correct fix: don't read the state here at all. Instead, pass it into a graphicsLayer - // lambda where only the draw phase is invalidated: - // Modifier.graphicsLayer { alpha = animatedAlpha } + // Instead, alphaState.value is read only inside the graphicsLayer lambda below, + // which runs in the draw phase. Only the GPU layer is invalidated on each tick — + // composition and layout are never touched. val infiniteTransition = rememberInfiniteTransition(label = "pulse_${item.id}") - val animatedAlpha by infiniteTransition.animateFloat( - initialValue = 0.50f, - targetValue = 1.00f, + val alphaState = infiniteTransition.animateFloat( + initialValue = 0.5f, + targetValue = 1.0f, animationSpec = infiniteRepeatable( - // Stagger durations slightly so items don't all flash in sync. - animation = tween(durationMillis = 600 + (item.id % 10) * 80, easing = LinearEasing), + animation = tween(durationMillis = 600 + (item.id % 10) * 80, easing = LinearEasing), repeatMode = RepeatMode.Reverse, ), label = "alpha_${item.id}", ) - // BAD: new Color object allocated on every recomposition — should be a top-level - // constant or wrapped in remember { Color(...) }. - val accentColor = Color( - red = (item.id * 37 % 200 + 55) / 255f, - green = (item.id * 71 % 180 + 50) / 255f, - blue = (item.id * 13 % 220 + 35) / 255f, - ) + // FIX — DeferredTargetAnimation for expand/collapse: + // Drives the body's height from 0→full (expand) or full→0 (collapse) entirely + // from inside Modifier.layout. State reads in a layout lambda trigger a layout-phase + // re-run, not a recomposition — so the 80 frames of a spring animation produce + // 80 layout passes and zero recompositions. + val expandAnim = remember { DeferredTargetAnimation(Float.VectorConverter) } Column( modifier = Modifier .fillMaxWidth() - // BAD 1: animateContentSize on every item in the list. - // When any card expands, the LayoutModifier runs its size interpolation on - // every animation frame for every card that has this modifier — not just the - // one the user tapped. - .animateContentSize() - // BAD 2: Modifier.alpha() evaluates its argument during composition, so the - // state read above makes this whole subtree recompose every frame. - // Modifier.graphicsLayer { alpha = animatedAlpha } would confine the read to - // the draw phase and skip recomposition entirely. - .alpha(animatedAlpha) + // FIX — graphicsLayer for alpha: alphaState.value is read inside the + // lambda, scoped to the draw phase. The composable tree above this point + // is never invalidated by the animation. + .graphicsLayer { alpha = alphaState.value } .background(accentColor.copy(alpha = 0.07f)) - .clickable { expanded = !expanded } - .padding(horizontal = 16.dp, vertical = 12.dp), + .clickable { expanded = !expanded }, ) { + // ── Header (always visible) ─────────────────────────────────────────── Row( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 12.dp), horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically, + verticalAlignment = Alignment.CenterVertically, ) { Column(modifier = Modifier.weight(1f)) { Text( - text = item.title, + text = item.title, style = MaterialTheme.typography.titleSmall, color = accentColor, ) Text( - text = item.subtitle, + text = item.subtitle, style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.outline, ) } Text( - text = if (expanded) "▲" else "▼", + text = if (expanded) "▲" else "▼", style = MaterialTheme.typography.labelMedium, color = accentColor, ) } - if (expanded) { + // ── Body (height animated via DeferredTargetAnimation) ──────────────── + // + // The body is always present in the composition tree — no if/else branch. + // Removing the branch means tapping the card causes exactly ONE recomposition + // (to flip the chevron); thereafter the spring animation runs purely in the + // layout phase via DeferredTargetAnimation. + // + // Modifier.layout measures the full body height, then reports an animated + // fraction of that height to the parent. clipToBounds() hides content that + // extends beyond the currently animated bounds so nothing visually overflows. + Column( + modifier = Modifier + .fillMaxWidth() + .clipToBounds() + .layout { measurable, constraints -> + val placeable = measurable.measure(constraints) + + // updateTarget() is called here (layout phase) — it starts or + // redirects the animation coroutine when the target changes, and + // returns the current interpolated progress value. + // Reading it here causes layout-phase invalidation on each frame, + // NOT a recomposition. + val progress = expandAnim.updateTarget( + target = if (expanded) 1f else 0f, + coroutineScope = scope, + animationSpec = spring(stiffness = Spring.StiffnessMediumLow), + ) + val animatedHeight = (placeable.height * progress) + .roundToInt() + .coerceAtLeast(0) + + layout(placeable.width, animatedHeight) { + placeable.place(0, 0) + } + }, + ) { Spacer(modifier = Modifier.height(8.dp)) Text(text = item.body, style = MaterialTheme.typography.bodySmall) Spacer(modifier = Modifier.height(4.dp)) - // Extra lines increase the layout cost when animateContentSize runs. repeat(4) { line -> Text( - text = "Detail line ${line + 1} — item #${item.id}", + text = "Detail line ${line + 1} — item #${item.id}", style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.outline, ) } + Spacer(modifier = Modifier.height(8.dp)) } } } diff --git a/ui/src/main/java/com/aquib/androidperflab/ui/DetailScreen.kt b/ui/src/main/java/com/aquib/androidperflab/ui/DetailScreen.kt index 8306dba..aeaf208 100644 --- a/ui/src/main/java/com/aquib/androidperflab/ui/DetailScreen.kt +++ b/ui/src/main/java/com/aquib/androidperflab/ui/DetailScreen.kt @@ -1,6 +1,5 @@ package com.aquib.androidperflab.ui -import android.annotation.SuppressLint import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -22,7 +21,9 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -41,26 +42,31 @@ import java.text.SimpleDateFormat import java.util.Date import java.util.Locale -@SuppressLint("UnrememberedMutableState") +// FIX — constant Color: moved out of DetailAuthorCard so the Color object is allocated +// once at class-load time rather than on every recomposition of the card. +private val AvatarColor = Color(0xFF1565C0) + @Composable fun DetailScreen( item: FeedItem, onBack: () -> Unit, modifier: Modifier = Modifier, ) { - // The only remember{} in this composable — drives continuous recompositions so every - // bad practice below has a visible cost during profiling. var tick by remember { mutableIntStateOf(0) } LaunchedEffect(Unit) { while (true) { delay(500L); tick++ } } - // BAD: mutableStateOf without remember — both values reset to 0 on every recomposition, - // so user interactions never persist across a recompose cycle. - var likeCount by mutableStateOf(0) - var bookmarkCount by mutableStateOf(0) + // FIX — remember state: both counters now survive tick-driven recompositions. + // Without remember{}, mutableStateOf creates a brand-new State object every + // recompose, so any increment the user performed was silently discarded the moment + // the next 500 ms tick fired. + var likeCount by remember { mutableStateOf(0) } + var bookmarkCount by remember { mutableStateOf(0) } - // BAD: no derivedStateOf — isPopular recalculates on every recomposition of DetailScreen - // (e.g. each tick), not only when likeCount actually changes. - val isPopular = likeCount > 50 + // FIX — derivedStateOf: isPopular is re-evaluated only when likeCount changes, + // and it only notifies observers (DetailTitle) when the resulting Boolean actually + // flips. Previously this bare comparison ran unconditionally on every tick, causing + // DetailTitle to recompose at 2 Hz even though isPopular was always false. + val isPopular by remember { derivedStateOf { likeCount > 50 } } Column( modifier = modifier @@ -69,39 +75,44 @@ fun DetailScreen( .padding(16.dp), verticalArrangement = Arrangement.spacedBy(12.dp), ) { - DetailBackButton(onBack = onBack) // 1 BAD: inline lambda - DetailHeroImage(url = item.imageUrl, tick = tick) // 2 recomposes every 500 ms - DetailTimestamp(millis = item.timestampMillis) // 3 BAD: inline SimpleDateFormat - DetailTitle(title = item.title, isPopular = isPopular) // 4 re-evaluates each tick - DetailTagsRow(title = item.title) // 5 BAD: split() inline - DetailReadingTime(description = item.description) // 6 BAD: word count inline - DetailLiveCounter(tick = tick) // 7 ticks every 500 ms - DetailInteractionBar( // 8 BAD: inline lambdas + DetailBackButton(onBack = onBack) + // FIX — split composables: the former DetailHeroImage(url, tick) bundled a + // stable image with a dynamic counter. Splitting them means the AsyncImage node + // is now skipped on every tick; only DetailLiveUpdateBadge recomposes at 2 Hz. + DetailHeroImage(url = item.imageUrl) + DetailLiveUpdateBadge(tick = tick) + DetailTimestamp(millis = item.timestampMillis) + DetailTitle(title = item.title, isPopular = isPopular) + DetailTagsRow(title = item.title) + DetailReadingTime(description = item.description) + DetailLiveCounter(tick = tick) + DetailInteractionBar( likeCount = likeCount, bookmarkCount = bookmarkCount, onLike = { likeCount++ }, onBookmark = { bookmarkCount++ }, ) - DetailDescription(description = item.description) // 9 - DetailStatsGrid(id = item.id) // 10 BAD: arithmetic inline - DetailRelatedSection(sourceId = item.id) // 11 BAD: no key, inline List - DetailAuthorCard(author = item.author, id = item.id) // 12 BAD: inline Color + ops + DetailDescription(description = item.description) + DetailStatsGrid(id = item.id) + DetailRelatedSection(sourceId = item.id) + DetailAuthorCard(author = item.author, id = item.id) } } -// ── Child composables ──────────────────────────────────────────────────────── +// ── Child composables ───────────────────────────────────────────────────────── @Composable private fun DetailBackButton(onBack: () -> Unit) { - // BAD: onBack is an inline lambda at the call site — a new instance each recomposition, - // so Compose cannot skip recomposing this child. TextButton(onClick = onBack) { Text("← Back to feed") } } +// FIX — stable image node: url is the only parameter and String is stable, so the +// Compose compiler marks this function skippable. The AsyncImage is never re-entered +// during tick-driven recompositions as long as the URL has not changed. @Composable -private fun DetailHeroImage(url: String, tick: Int) { +private fun DetailHeroImage(url: String) { AsyncImage( model = url, contentDescription = null, @@ -111,6 +122,12 @@ private fun DetailHeroImage(url: String, tick: Int) { .height(220.dp) .clip(RoundedCornerShape(12.dp)), ) +} + +// FIX — extracted dynamic node: separated from DetailHeroImage so only this +// lightweight Text recomposes every 500 ms. The image above it is fully isolated. +@Composable +private fun DetailLiveUpdateBadge(tick: Int) { Text( text = "Live updates: $tick", style = MaterialTheme.typography.labelSmall, @@ -120,9 +137,13 @@ private fun DetailHeroImage(url: String, tick: Int) { @Composable private fun DetailTimestamp(millis: Long) { - // BAD: new SimpleDateFormat and Date allocated on every recomposition. - val formatted = SimpleDateFormat("EEEE, MMMM dd yyyy 'at' HH:mm:ss", Locale.getDefault()) - .format(Date(millis)) + // FIX — remember(millis): SimpleDateFormat and Date are allocated only when the + // timestamp value actually changes. Previously both were allocated on every + // recomposition even though the formatted string was always identical. + val formatted = remember(millis) { + SimpleDateFormat("EEEE, MMMM dd yyyy 'at' HH:mm:ss", Locale.getDefault()) + .format(Date(millis)) + } Text( text = formatted, style = MaterialTheme.typography.labelMedium, @@ -156,30 +177,36 @@ private fun DetailTitle(title: String, isPopular: Boolean) { @Composable private fun DetailTagsRow(title: String) { - // BAD: split + filter + map run on every recomposition — should be remember { }. - val tags = title.split(" ").filter { it.length > 3 } + // FIX — remember(title): split + filter run only when title changes. + // FIX — key(tag): each chip now has a stable identity so Compose can diff + // additions and removals rather than destroying and recreating every chip. + val tags = remember(title) { title.split(" ").filter { it.length > 3 } } Row(horizontalArrangement = Arrangement.spacedBy(4.dp)) { - // BAD: no key {} — Compose cannot track tag identity across recompositions. tags.forEach { tag -> - Text( - text = "#${tag.lowercase()}", - style = MaterialTheme.typography.labelSmall, - modifier = Modifier - .background( - MaterialTheme.colorScheme.secondaryContainer, - RoundedCornerShape(4.dp), - ) - .padding(horizontal = 6.dp, vertical = 2.dp), - ) + key(tag) { + Text( + text = "#${tag.lowercase()}", + style = MaterialTheme.typography.labelSmall, + modifier = Modifier + .background( + MaterialTheme.colorScheme.secondaryContainer, + RoundedCornerShape(4.dp), + ) + .padding(horizontal = 6.dp, vertical = 2.dp), + ) + } } } } @Composable private fun DetailReadingTime(description: String) { - // BAD: split + size called on every recomposition — should be remember { }. - val wordCount = description.split(" ").size - val minutes = (wordCount / 200).coerceAtLeast(1) + // FIX — remember(description): split().size runs only when description changes. + // The result is a Pair so both derived values share a single remember block. + val (wordCount, minutes) = remember(description) { + val words = description.split(" ").size + words to (words / 200).coerceAtLeast(1) + } Text( text = "$wordCount words · $minutes min read", style = MaterialTheme.typography.bodySmall, @@ -203,13 +230,12 @@ private fun DetailInteractionBar( onLike: () -> Unit, onBookmark: () -> Unit, ) { - // BAD: no derivedStateOf — these strings are rebuilt every recomposition even when - // likeCount / bookmarkCount have not changed (e.g. on each tick). - val likeLabel = "♥ Like ($likeCount)" - val bookmarkLabel = "🔖 Save ($bookmarkCount)" + // FIX — remember(count): label strings are memoized per count value. Previously + // concatenation ran on every recomposition even when counts had not changed + // (e.g. during a tick-triggered recompose of the parent). + val likeLabel = remember(likeCount) { "♥ Like ($likeCount)" } + val bookmarkLabel = remember(bookmarkCount) { "🔖 Save ($bookmarkCount)" } Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - // BAD: onLike / onBookmark are inline lambdas — new instances each recomposition, - // preventing Compose from skipping Button recomposition. Button(onClick = onLike, modifier = Modifier.testTag("detail_like_button")) { Text(likeLabel) } OutlinedButton(onClick = onBookmark, modifier = Modifier.testTag("detail_bookmark_button")) { Text(bookmarkLabel) } } @@ -222,11 +248,12 @@ private fun DetailDescription(description: String) { @Composable private fun DetailStatsGrid(id: Int) { - // BAD: all multiplications run on every recomposition — should be remember { }. - val views = id * 317 + 1_200 - val shares = id * 41 + 80 - val comments = id * 13 + 25 - val reposts = id * 7 + 10 + // FIX — remember(id): all four stats are pure functions of id, which never + // changes for a given item. Previously the multiplications ran on every recompose. + val views = remember(id) { id * 317 + 1_200 } + val shares = remember(id) { id * 41 + 80 } + val comments = remember(id) { id * 13 + 25 } + val reposts = remember(id) { id * 7 + 10 } Column(verticalArrangement = Arrangement.spacedBy(4.dp)) { Text("Post stats", style = MaterialTheme.typography.titleSmall) Row(horizontalArrangement = Arrangement.spacedBy(16.dp)) { @@ -242,19 +269,20 @@ private fun DetailStatsGrid(id: Int) { @Composable private fun DetailRelatedSection(sourceId: Int) { - // BAD: List allocation on every recomposition — should be remember { }. - val relatedIds = List(8) { i -> (sourceId + i + 1) % 220 } - + // FIX — remember(sourceId): list allocation runs only when sourceId changes. + // FIX — key(relatedId): each row has a stable identity so Compose diffs the + // eight-item block instead of recomposing it wholesale. + val relatedIds = remember(sourceId) { List(8) { i -> (sourceId + i + 1) % 220 } } Column(verticalArrangement = Arrangement.spacedBy(4.dp)) { Text("Related posts", style = MaterialTheme.typography.titleSmall) - // BAD: no key {} on forEach — Compose cannot identify items across recompositions, - // so it may recompose all children even when only one changes. relatedIds.forEach { relatedId -> - DetailRelatedItem( - imageUrl = "https://picsum.photos/seed/$relatedId/60/60", - title = "Related Post #$relatedId", - category = "Category ${relatedId % 10}", - ) + key(relatedId) { + DetailRelatedItem( + imageUrl = "https://picsum.photos/seed/$relatedId/60/60", + title = "Related Post #$relatedId", + category = "Category ${relatedId % 10}", + ) + } } } } @@ -285,8 +313,15 @@ private fun DetailRelatedItem(imageUrl: String, title: String, category: String) @Composable private fun DetailAuthorCard(author: String, id: Int) { - // BAD: Color object allocated inline — should be a top-level constant or remember { }. - val avatarColor = Color(0xFF1565C0) + // FIX — remember(author): initials derivation (split + map + joinToString) runs + // only when the author string changes, not on every recomposition. + val initials = remember(author) { + author.split(" ") + .mapNotNull { it.firstOrNull()?.toString() } + .joinToString("") + } + // FIX — remember(id): post-count string is a pure function of id. + val postsPublished = remember(id) { "${id % 50 + 5} posts published" } Row( modifier = Modifier @@ -296,23 +331,17 @@ private fun DetailAuthorCard(author: String, id: Int) { horizontalArrangement = Arrangement.spacedBy(12.dp), verticalAlignment = Alignment.CenterVertically, ) { - // BAD: split + map + joinToString to derive initials on every recomposition. - val initials = author.split(" ") - .mapNotNull { it.firstOrNull()?.toString() } - .joinToString("") - Box( modifier = Modifier .size(40.dp) - .background(avatarColor, CircleShape), + .background(AvatarColor, CircleShape), contentAlignment = Alignment.Center, ) { Text(text = initials, color = Color.White, style = MaterialTheme.typography.labelLarge) } Column { Text(author, style = MaterialTheme.typography.bodyMedium, fontWeight = FontWeight.SemiBold) - // BAD: inline modulo + addition on every recomposition. - Text("${id % 50 + 5} posts published", style = MaterialTheme.typography.labelSmall) + Text(postsPublished, style = MaterialTheme.typography.labelSmall) } } } diff --git a/ui/src/main/java/com/aquib/androidperflab/ui/FeedItem.kt b/ui/src/main/java/com/aquib/androidperflab/ui/FeedItem.kt index d6b7daf..c133e5c 100644 --- a/ui/src/main/java/com/aquib/androidperflab/ui/FeedItem.kt +++ b/ui/src/main/java/com/aquib/androidperflab/ui/FeedItem.kt @@ -1,5 +1,14 @@ package com.aquib.androidperflab.ui +import androidx.compose.runtime.Immutable + +// @Immutable tells the Compose compiler that every field in FeedItem is immutable after +// construction. This lets Compose skip any composable whose only changed parameter is +// a FeedItem whose reference is equal to the previous value, without re-checking each +// field individually. Use @Immutable (not @Stable) because all fields are val and all +// field types are themselves immutable — a stronger guarantee than @Stable, which only +// promises that mutations will be notified. +@Immutable data class FeedItem( val id: Int, val title: String, diff --git a/ui/src/main/java/com/aquib/androidperflab/ui/FeedScreen.kt b/ui/src/main/java/com/aquib/androidperflab/ui/FeedScreen.kt index f87d857..1ff94b9 100644 --- a/ui/src/main/java/com/aquib/androidperflab/ui/FeedScreen.kt +++ b/ui/src/main/java/com/aquib/androidperflab/ui/FeedScreen.kt @@ -50,6 +50,10 @@ fun FeedScreen( ) { val items = remember { generateFeedItems() } LazyColumn(modifier = modifier.fillMaxSize()) { + // FIX 1 — stable key: key = { it.id } pins each slot to a specific FeedItem by + // identity. Without a key, inserting or removing one item causes every subsequent + // row to be destroyed and recreated. With a stable key Compose reuses existing + // nodes and only recomposes the slots whose content actually changed. items(items = items, key = { it.id }) { item -> FeedItemRow(item = item, onClick = { onItemClick(item) }) HorizontalDivider() @@ -59,10 +63,13 @@ fun FeedScreen( @Composable private fun FeedItemRow(item: FeedItem, onClick: () -> Unit = {}) { - // Intentionally unoptimized: a new SimpleDateFormat is allocated and a new Date is - // constructed on every recomposition instead of being computed once with remember {}. - val timestamp = SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss", Locale.getDefault()) - .format(Date(item.timestampMillis)) + // FIX 2 — remembered timestamp: SimpleDateFormat and Date are only allocated when + // item.timestampMillis changes. Previously they were allocated on every recomposition, + // even though the formatted string never changed between recompose passes. + val timestamp = remember(item.timestampMillis) { + SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss", Locale.getDefault()) + .format(Date(item.timestampMillis)) + } Row( modifier = Modifier @@ -71,14 +78,8 @@ private fun FeedItemRow(item: FeedItem, onClick: () -> Unit = {}) { .padding(horizontal = 16.dp, vertical = 10.dp), horizontalArrangement = Arrangement.spacedBy(12.dp), ) { - AsyncImage( - model = item.imageUrl, - contentDescription = null, - contentScale = ContentScale.Crop, - modifier = Modifier - .size(72.dp) - .clip(RoundedCornerShape(8.dp)), - ) + // FIX 3 — stable image wrapper: see FeedItemImage below. + FeedItemImage(url = item.imageUrl) Column(verticalArrangement = Arrangement.spacedBy(2.dp)) { Text(text = item.title, style = MaterialTheme.typography.titleSmall) Text(text = item.subtitle, style = MaterialTheme.typography.labelMedium) @@ -92,3 +93,20 @@ private fun FeedItemRow(item: FeedItem, onClick: () -> Unit = {}) { } } } + +// FIX 3 — stable AsyncImage wrapper: String is a stable type, so the Compose compiler +// marks this function as skippable. When FeedItemRow recomposes for any reason other +// than a URL change (e.g. a new onClick lambda instance), Compose compares the url +// argument and skips the entire function body — AsyncImage is never re-entered and +// no unnecessary image-load checks are issued to the Coil cache. +@Composable +private fun FeedItemImage(url: String) { + AsyncImage( + model = url, + contentDescription = null, + contentScale = ContentScale.Crop, + modifier = Modifier + .size(72.dp) + .clip(RoundedCornerShape(8.dp)), + ) +}