feat: integrate Baseline Profiles for performance optimization#135
feat: integrate Baseline Profiles for performance optimization#135markrizkalla wants to merge 2 commits intoopenMF:devfrom
Conversation
- Create a new `:baselineprofile` module to handle profile generation and benchmarking. - Implement `BaselineProfileGenerator` to capture app startup flows using `BaselineProfileRule`. - Implement `StartupBenchmarks` using `MacrobenchmarkRule` to measure and verify startup performance improvements. - Configure the `androidx.baselineprofile` plugin across the project and link the new module to `:cmp-android`. - Update dependencies in `libs.versions.toml` to include `benchmark-macro-junit4`, `uiautomator`, and `espresso-core`. - Register the `:baselineprofile` module in `settings.gradle.kts`.
📝 WalkthroughWalkthroughThis PR introduces a new baseline profile module to the project for performance measurement and optimization. It adds instrumentation tests for generating baseline profiles and benchmarking startup performance, along with necessary Gradle configuration, dependencies, and manifest setup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@markrizkalla It looks like the scope of this PR is limited to Android app only. I think it should be mentioned in the PR title and description. Edit: I noticed that the description mentions Are there any metrics related to the optimization and runtime performance that can be showcased here? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
gradle/libs.versions.toml (1)
115-115: Optional DRY cleanup: reuse the existing macrobenchmark version key.
benchmarkMacroJunit4currently mirrorsandroidxMacroBenchmark(1.4.1). Reusing one key reduces future version drift risk.♻️ Proposed refactor
- benchmarkMacroJunit4 = "1.4.1" - androidx-benchmark-macro-junit4 = { module = "androidx.benchmark:benchmark-macro-junit4", version.ref = "benchmarkMacroJunit4" } + androidx-benchmark-macro-junit4 = { module = "androidx.benchmark:benchmark-macro-junit4", version.ref = "androidxMacroBenchmark" }Also applies to: 337-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle/libs.versions.toml` at line 115, The duplicate version key benchmarkMacroJunit4 duplicates androidxMacroBenchmark (both "1.4.1"); change benchmarkMacroJunit4 to reference the existing androidxMacroBenchmark version key instead of hardcoding the same literal so updates are centralized—locate the benchmarkMacroJunit4 entry and replace its value with a reference to androidxMacroBenchmark (so only androidxMacroBenchmark holds the canonical version).baselineProfile/src/main/java/org/mifos/baselineprofile/StartupBenchmarks.kt (1)
66-78: Consider implementing the fully-drawn tracking mentioned in the TODO.The TODO indicates that interactions should be added to wait for
Activity.reportFullyDrawn. This is important for accurate Time To Full Display (TTFD) metrics beyond just Time To Initial Display (TTID). Without this, the benchmark only measures initial frame rendering, not when the app is actually usable.Would you like me to help generate code to wait for fully-drawn state? If you're using Jetpack Compose, I can provide an example using
ReportDrawnWhenor similar APIs. Alternatively, I can open a follow-up issue to track this enhancement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baselineProfile/src/main/java/org/mifos/baselineprofile/StartupBenchmarks.kt` around lines 66 - 78, The measureBlock currently only calls startActivityAndWait and lacks waiting for Activity.reportFullyDrawn; update measureBlock to wait for the app's fully-drawn signal (so TTFD is measured) by adding a post-launch wait that detects when reportFullyDrawn has occurred — either by emitting a test-only signal from the app (e.g., set a specific view/contentDescription or a debug-only id when Activity.reportFullyDrawn is called) and using UiAutomator to wait for that element, or by integrating Jetpack Compose's ReportDrawnWhen/ReportDrawnAfter in your UI code and waiting for that condition after startActivityAndWait; ensure the test references the chosen signal and only proceeds when the fully-drawn marker is observed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@baselineProfile/build.gradle.kts`:
- Around line 50-57: The current onVariants block may put nullable
applicationIds into instrumentationRunnerArguments via v.testedApks.map {
artifactsLoader.load(it)?.applicationId }; change this to handle nulls by using
v.testedApks.mapNotNull { artifactsLoader.load(it)?.applicationId } and only
call v.instrumentationRunnerArguments.put("targetAppId", ...) when the resulting
collection is not empty (e.g., joinToString(",") for multiple ids) or provide a
safe default (empty string) to avoid inserting nulls; update references in the
androidComponents/onVariants block (v, artifactsLoader, getBuiltArtifactsLoader,
testedApks, load, applicationId, instrumentationRunnerArguments) accordingly.
In `@settings.gradle.kts`:
- Line 92: The included module name casing is wrong: change the
include(":baselineprofile") entry to match the actual directory name (use
":baselineProfile") or alternatively add an explicit projectDir mapping for
project(":baselineprofile") to file("baselineProfile"); update the
settings.gradle.kts entry so the module identifier and the filesystem directory
casing match (or are explicitly mapped) to avoid resolution failures on
case-sensitive CI.
---
Nitpick comments:
In
`@baselineProfile/src/main/java/org/mifos/baselineprofile/StartupBenchmarks.kt`:
- Around line 66-78: The measureBlock currently only calls startActivityAndWait
and lacks waiting for Activity.reportFullyDrawn; update measureBlock to wait for
the app's fully-drawn signal (so TTFD is measured) by adding a post-launch wait
that detects when reportFullyDrawn has occurred — either by emitting a test-only
signal from the app (e.g., set a specific view/contentDescription or a
debug-only id when Activity.reportFullyDrawn is called) and using UiAutomator to
wait for that element, or by integrating Jetpack Compose's
ReportDrawnWhen/ReportDrawnAfter in your UI code and waiting for that condition
after startActivityAndWait; ensure the test references the chosen signal and
only proceeds when the fully-drawn marker is observed.
In `@gradle/libs.versions.toml`:
- Line 115: The duplicate version key benchmarkMacroJunit4 duplicates
androidxMacroBenchmark (both "1.4.1"); change benchmarkMacroJunit4 to reference
the existing androidxMacroBenchmark version key instead of hardcoding the same
literal so updates are centralized—locate the benchmarkMacroJunit4 entry and
replace its value with a reference to androidxMacroBenchmark (so only
androidxMacroBenchmark holds the canonical version).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da7a9f27-7d42-4342-ac32-4e40117226c7
📒 Files selected for processing (10)
baselineProfile/.gitignorebaselineProfile/build.gradle.ktsbaselineProfile/src/main/AndroidManifest.xmlbaselineProfile/src/main/java/org/mifos/baselineprofile/BaselineProfileGenerator.ktbaselineProfile/src/main/java/org/mifos/baselineprofile/StartupBenchmarks.ktbuild.gradle.ktscmp-android/build.gradle.ktsgradle.propertiesgradle/libs.versions.tomlsettings.gradle.kts
| androidComponents { | ||
| onVariants { v -> | ||
| val artifactsLoader = v.artifacts.getBuiltArtifactsLoader() | ||
| v.instrumentationRunnerArguments.put( | ||
| "targetAppId", | ||
| v.testedApks.map { artifactsLoader.load(it)?.applicationId } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Handle potential null applicationId gracefully.
The expression artifactsLoader.load(it)?.applicationId can yield null if the artifacts fail to load or the applicationId is unavailable. Passing a null value to instrumentationRunnerArguments would cause the benchmark to fail silently or with an unclear error at runtime.
🛡️ Proposed fix to add null handling
androidComponents {
onVariants { v ->
val artifactsLoader = v.artifacts.getBuiltArtifactsLoader()
v.instrumentationRunnerArguments.put(
"targetAppId",
- v.testedApks.map { artifactsLoader.load(it)?.applicationId }
+ v.testedApks.map { apks ->
+ artifactsLoader.load(apks)?.applicationId
+ ?: error("Failed to load applicationId from tested APKs for variant ${v.name}")
+ }
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| androidComponents { | |
| onVariants { v -> | |
| val artifactsLoader = v.artifacts.getBuiltArtifactsLoader() | |
| v.instrumentationRunnerArguments.put( | |
| "targetAppId", | |
| v.testedApks.map { artifactsLoader.load(it)?.applicationId } | |
| ) | |
| } | |
| androidComponents { | |
| onVariants { v -> | |
| val artifactsLoader = v.artifacts.getBuiltArtifactsLoader() | |
| v.instrumentationRunnerArguments.put( | |
| "targetAppId", | |
| v.testedApks.map { apks -> | |
| artifactsLoader.load(apks)?.applicationId | |
| ?: error("Failed to load applicationId from tested APKs for variant ${v.name}") | |
| } | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baselineProfile/build.gradle.kts` around lines 50 - 57, The current
onVariants block may put nullable applicationIds into
instrumentationRunnerArguments via v.testedApks.map {
artifactsLoader.load(it)?.applicationId }; change this to handle nulls by using
v.testedApks.mapNotNull { artifactsLoader.load(it)?.applicationId } and only
call v.instrumentationRunnerArguments.put("targetAppId", ...) when the resulting
collection is not empty (e.g., joinToString(",") for multiple ids) or provide a
safe default (empty string) to avoid inserting nulls; update references in the
androidComponents/onVariants block (v, artifactsLoader, getBuiltArtifactsLoader,
testedApks, load, applicationId, instrumentationRunnerArguments) accordingly.
| """.trimIndent() | ||
| } No newline at end of file | ||
| } | ||
| include(":baselineprofile") |
There was a problem hiding this comment.
Module include path casing likely breaks project resolution on case-sensitive environments.
Line 92 includes :baselineprofile, but the checked-in module directory is baselineProfile (capital P). Without explicit projectDir mapping, Gradle may fail to locate the module on Linux CI.
🐛 Proposed fix
include(":baselineprofile")
+project(":baselineprofile").projectDir = file("baselineProfile")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| include(":baselineprofile") | |
| include(":baselineprofile") | |
| project(":baselineprofile").projectDir = file("baselineProfile") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@settings.gradle.kts` at line 92, The included module name casing is wrong:
change the include(":baselineprofile") entry to match the actual directory name
(use ":baselineProfile") or alternatively add an explicit projectDir mapping for
project(":baselineprofile") to file("baselineProfile"); update the
settings.gradle.kts entry so the module identifier and the filesystem directory
casing match (or are explicitly mapped) to avoid resolution failures on
case-sensitive CI.
I will provide more details |
- Generated startup-prof.txt (DEX layout optimization rules)
2 participants
:baselineprofilemodule to handle profile generation and benchmarking.BaselineProfileGeneratorto capture app startup flows usingBaselineProfileRule.StartupBenchmarksusingMacrobenchmarkRuleto measure and verify startup performance improvements.androidx.baselineprofileplugin across the project and link the new module to:cmp-android.libs.versions.tomlto includebenchmark-macro-junit4,uiautomator, andespresso-core.:baselineprofilemodule insettings.gradle.kts.Changes:
Usage:
Generate: ./gradlew :cmp-android:generateProdReleaseBaselineProfile
Benchmark: ./gradlew :baselineprofile:connectedProdBenchmarkReleaseAndroidTest"
The metrics is calculated after 10 iterations
Without Baseline Profile (CV = 23%):
Run 1: 391ms ████████████████████████████████████████
Run 2: 444ms █████████████████████████████████████████████
Run 3: 405ms █████████████████████████████████████████
Run 4: 345ms ███████████████████████████████████
Run 5: 348ms ███████████████████████████████████
Run 6: 241ms ████████████████████████
Run 7: 255ms █████████████████████████
Run 8: 328ms █████████████████████████████████
Run 9: 253ms █████████████████████████
Run 10: 238ms ████████████████████████
With Baseline Profile (CV = 10%):
Run 1: 240ms ████████████████████████
Run 2: 278ms ████████████████████████████
Run 3: 306ms ██████████████████████████████
Run 4: 258ms ██████████████████████████
Run 5: 249ms █████████████████████████
Run 6: 224ms ██████████████████████
Run 7: 231ms ███████████████████████
Run 8: 233ms ███████████████████████
Run 9: 240ms ████████████████████████
Run 10: 260ms ██████████████████████████
┌──────────────────────────────────────────────────────────────────┐
│ │
│ 📊 Startup Benchmark Results — Pixel 6a (Android 16) │
│ │
│ ┌────────────────────┬──────────────┬──────────────┬─────────┐ │
│ │ Metric │ No Profile │ With Profile │ Change │ │
│ ├────────────────────┼──────────────┼──────────────┼─────────┤ │
│ │ Median Startup │ 336.34 ms │ 244.74 ms │ -27.2% │ │
│ │ Minimum Startup │ 237.95 ms │ 224.41 ms │ -5.7% │ │
│ │ Maximum Startup │ 444.33 ms │ 306.02 ms │ -31.1% │ │
│ │ Consistency (CV) │ 23.10% │ 9.89% │ 2.3x ⬆️ │ │
│ └────────────────────┴──────────────┴──────────────┴─────────┘ │
│ │
│ No Profile: ████████████████████████████████████ 336ms │
│ With Profile: █████████████████████████ 245ms │
│ ^^^^^^^^^^^ │
│ 91ms SAVED │
│ 27% FASTER │
│ │
└──────────────────────────────────────────────────────────────────┘
Summary by CodeRabbit
New Features
Chores