chore: project maintenance audit (build fixes, capability wiring, results-doc corrections, ComponentActivity parity) - #26
Merged
Conversation
runBenchmarkCompose/View/All depended on task paths that didn't exist (:benchmark:benchmarkComposeRun was never defined; benchmarkViewRun is registered on the root project, not :benchmark), so --dry-run failed with "Task not found". Add the missing benchmarkComposeRun task and correct the task references throughout.
…enchmarks sanitizedMetricsForBenchmark()/measureStartupWithCapabilityReport() lived in benchmark/src/androidTest/.../BenchmarkUtils.kt, which is dead code for this com.android.test module (only src/main is compiled into the instrumentation APK) and was never called from anywhere reachable. It also imported androidx.benchmark.macro.MacrobenchmarkRule instead of the .junit4 variant actually used by the test classes -- likely why it was never wired in to begin with. Move the capability-report logic into the real src/main/BenchmarkUtils.kt with the import fixed, add the kotlinx-serialization-json dependency the JSON logging needs (:shared only exposes it as `implementation`, not transitive), and switch coldStartup_compose()/coldStartup_view() to call measureStartupWithCapabilityReport() instead of hardcoding a metrics list. Delete the now-redundant dead file. Fast-scroll tests and SmokeBenchmark.kt are left as-is; their custom swipe measureBlock doesn't fit this cold-start-shaped helper.
results/README.md's stated policy is curated summaries/manifests only, but the raw *-benchmarkData.json and additionaltestoutput.*.txt dumps were sitting untracked in results/ with nothing stopping a future `git add -A` from committing ~370KB of raw output. Add explicit ignore patterns so the policy actually holds.
…07-06) README.md and results/run-manifest-2026-08-07.md cited an emulator run on 2026-08-07, committed by f582b68 on 2026-07-09 -- a month before its own claimed run date. The raw perfetto-trace filenames and artifact mtimes retained under results/ show the run actually happened on 2026-07-06. Rename the manifest and update references accordingly; the committed numbers are unchanged, only the date/filenames were wrong. Also cross-reference this run against benchmark-results.md's separate 2026-07-23 run so the two results docs read as distinct dated data points instead of unrelated, conflicting claims.
Mark the Gradle task graph, MetricCapability wiring, results artifact policy, and run-date/provenance findings as resolved with what changed. Keep the exception-handling, nested-Gradle-invocation, and emulator-only findings open. Also correct the coldStartup_view() test description (no longer StartupTimingMetric-only) and flag a similarly-shaped forward-dated/inconsistent-host issue found in README's "Maintenance status" header that wasn't independently verified this pass.
…K-path claim Same source commit (f582b68) that mislabeled the benchmark run date also changed this header from 2026-05-31 to 2026-08-07, a date in the future relative to its own 2026-07-09 commit timestamp. Corrected to 2026-07-09, the only real evidence of when the text was written. Also drop the claim that local.properties points at a nonexistent macOS SDK path: local.properties is gitignored and per-developer by design, so it shouldn't have been asserted as a persistent repo fact -- and it's currently false in this environment (valid Linux SDK path; every Gradle command this session succeeded).
…arity app-view.MainActivity extended AppCompatActivity while app-compose.MainActivity uses the lighter ComponentActivity -- a real confound in a study whose core claim depends on a fair cold-start comparison (tracked as TODO #6 in benchmark-results.md). Nothing here uses AppCompat delegate behaviors (no ActionBar/Toolbar, no vector-drawable back-compat needed at minSdk 24), so Theme.MaterialComponents.DayNight.NoActionBar still resolves without it. Verified assembleBenchmark/assembleRelease (incl. lintVitalRelease) pass. Not re-benchmarked on a device -- the published cold-start numbers in benchmark-results.md/README.md were measured against the old AppCompatActivity version and may no longer be accurate.
fastScroll_compose() and fastScroll_view() had byte-for-byte identical swipe-gesture logic inlined in each measureBlock. Extract to BenchmarkUtils.performFastScrollGestures(). Pure code motion, no behavior change.
… steps Record the maintenance-status header fix, the ComponentActivity parity fix, the shared gesture-helper extraction, and a warning about an incorrect DiffUtil recommendation in the untracked REVIEW.md found in this checkout. Add re-benchmark-after-ComponentActivity-change and the Thread.sleep gesture timing to the still-open list.
There was a problem hiding this comment.
Pull request overview
This PR is a maintenance/audit pass for the Compose vs View performance benchmark project, focusing on fixing benchmark task wiring, integrating the existing MetricCapability model into cold-start benchmarks, and correcting results documentation (including run-date provenance) while improving Compose/View parity by aligning MainActivity base classes.
Changes:
- Fixed/extended root Gradle benchmark convenience tasks and updated benchmark module dependencies for capability-report JSON logging.
- Wired capability-based metric selection into
coldStartup_compose()/coldStartup_view()and extracted shared fast-scroll gestures. - Corrected results/docs metadata (run date, references) and updated View app parity (
ComponentActivity), plus ignored raw results artifacts.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| results/run-manifest-2026-07-06.md | Corrects the recorded run date and artifact paths, adds a correction note. |
| README.md | Updates maintenance status, fixes run references, and clarifies results provenance/cross-links. |
| CLAUDE.md | Updates benchmark source-set guidance and audit/known-issues documentation. |
| build.gradle.kts | Adds/fixes benchmark task graph and convenience tasks for running suites/classes. |
| benchmark/src/main/java/dev/egarcia/andperf/benchmark/ViewBenchmarks.kt | Uses capability-aware cold-start helper; reuses shared fast-scroll gestures. |
| benchmark/src/main/java/dev/egarcia/andperf/benchmark/ComposeBenchmarks.kt | Uses capability-aware cold-start helper; reuses shared fast-scroll gestures. |
| benchmark/src/main/java/dev/egarcia/andperf/benchmark/BenchmarkUtils.kt | Adds capability-report + metric sanitization helpers; extracts fast-scroll gesture helper. |
| benchmark/src/androidTest/java/dev/egarcia/andperf/benchmark/BenchmarkUtils.kt | Removes dead/uncompiled androidTest-side BenchmarkUtils implementation. |
| benchmark/build.gradle.kts | Adds direct kotlinx-serialization dependency needed for capability JSON logging. |
| benchmark-results.md | Notes separate earlier run and marks ComponentActivity parity TODO as completed. |
| app-view/src/main/java/dev/egarcia/andperf/view/MainActivity.kt | Switches View app to ComponentActivity for parity with Compose app. |
| .gitignore | Ignores raw per-run benchmark artifacts per results retention policy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
27
to
28
| /** Shared helpers for benchmarks. Keep this class minimal and free of instrumentation-specific | ||
| * side-effects so it can be used by multiple test classes. */ |
- CLAUDE.md: remove the "Known broken tasks" note for runBenchmarkCompose/View/All -- this PR fixes those tasks, so the note now contradicted the "Resolved" section later in the same file. - BenchmarkUtils.kt: update the class KDoc, which still said "minimal and free of instrumentation-specific side-effects" despite now containing Logcat capability-report emission and instrumentation-dependent helpers.
Pulls together what's left after this PR's audit pass into one prioritized, actionable list (P0: needs a device/emulator run; P1: safe code fixes; P2: research-quality polish; P3: minor). Matches the NEXT_STEPS.md convention already used elsewhere in this workspace (see JMW/game/NEXT_STEPS.md).
3 tasks
e-Garcia
added a commit
that referenced
this pull request
Aug 2, 2026
* fix(build): repair broken benchmark Gradle task graph runBenchmarkCompose/View/All depended on task paths that didn't exist (:benchmark:benchmarkComposeRun was never defined; benchmarkViewRun is registered on the root project, not :benchmark), so --dry-run failed with "Task not found". Add the missing benchmarkComposeRun task and correct the task references throughout. * feat(benchmark): wire MetricCapability architecture into cold-start benchmarks sanitizedMetricsForBenchmark()/measureStartupWithCapabilityReport() lived in benchmark/src/androidTest/.../BenchmarkUtils.kt, which is dead code for this com.android.test module (only src/main is compiled into the instrumentation APK) and was never called from anywhere reachable. It also imported androidx.benchmark.macro.MacrobenchmarkRule instead of the .junit4 variant actually used by the test classes -- likely why it was never wired in to begin with. Move the capability-report logic into the real src/main/BenchmarkUtils.kt with the import fixed, add the kotlinx-serialization-json dependency the JSON logging needs (:shared only exposes it as `implementation`, not transitive), and switch coldStartup_compose()/coldStartup_view() to call measureStartupWithCapabilityReport() instead of hardcoding a metrics list. Delete the now-redundant dead file. Fast-scroll tests and SmokeBenchmark.kt are left as-is; their custom swipe measureBlock doesn't fit this cold-start-shaped helper. * chore(results): gitignore raw per-run benchmark artifacts results/README.md's stated policy is curated summaries/manifests only, but the raw *-benchmarkData.json and additionaltestoutput.*.txt dumps were sitting untracked in results/ with nothing stopping a future `git add -A` from committing ~370KB of raw output. Add explicit ignore patterns so the policy actually holds. * fix(docs): correct mislabeled benchmark run date (2026-08-07 -> 2026-07-06) README.md and results/run-manifest-2026-08-07.md cited an emulator run on 2026-08-07, committed by f582b68 on 2026-07-09 -- a month before its own claimed run date. The raw perfetto-trace filenames and artifact mtimes retained under results/ show the run actually happened on 2026-07-06. Rename the manifest and update references accordingly; the committed numbers are unchanged, only the date/filenames were wrong. Also cross-reference this run against benchmark-results.md's separate 2026-07-23 run so the two results docs read as distinct dated data points instead of unrelated, conflicting claims. * docs(claude): update audit findings with resolved/open status Mark the Gradle task graph, MetricCapability wiring, results artifact policy, and run-date/provenance findings as resolved with what changed. Keep the exception-handling, nested-Gradle-invocation, and emulator-only findings open. Also correct the coldStartup_view() test description (no longer StartupTimingMetric-only) and flag a similarly-shaped forward-dated/inconsistent-host issue found in README's "Maintenance status" header that wasn't independently verified this pass. * fix(docs): correct README maintenance-status header date and stale SDK-path claim Same source commit (f582b68) that mislabeled the benchmark run date also changed this header from 2026-05-31 to 2026-08-07, a date in the future relative to its own 2026-07-09 commit timestamp. Corrected to 2026-07-09, the only real evidence of when the text was written. Also drop the claim that local.properties points at a nonexistent macOS SDK path: local.properties is gitignored and per-developer by design, so it shouldn't have been asserted as a persistent repo fact -- and it's currently false in this environment (valid Linux SDK path; every Gradle command this session succeeded). * fix(app-view): switch MainActivity to ComponentActivity for Compose parity app-view.MainActivity extended AppCompatActivity while app-compose.MainActivity uses the lighter ComponentActivity -- a real confound in a study whose core claim depends on a fair cold-start comparison (tracked as TODO #6 in benchmark-results.md). Nothing here uses AppCompat delegate behaviors (no ActionBar/Toolbar, no vector-drawable back-compat needed at minSdk 24), so Theme.MaterialComponents.DayNight.NoActionBar still resolves without it. Verified assembleBenchmark/assembleRelease (incl. lintVitalRelease) pass. Not re-benchmarked on a device -- the published cold-start numbers in benchmark-results.md/README.md were measured against the old AppCompatActivity version and may no longer be accurate. * refactor(benchmark): extract shared fast-scroll gesture helper fastScroll_compose() and fastScroll_view() had byte-for-byte identical swipe-gesture logic inlined in each measureBlock. Extract to BenchmarkUtils.performFastScrollGestures(). Pure code motion, no behavior change. * docs(claude): document this round's audit findings and remaining next steps Record the maintenance-status header fix, the ComponentActivity parity fix, the shared gesture-helper extraction, and a warning about an incorrect DiffUtil recommendation in the untracked REVIEW.md found in this checkout. Add re-benchmark-after-ComponentActivity-change and the Thread.sleep gesture timing to the still-open list. * docs: address Copilot review feedback on PR #26 - CLAUDE.md: remove the "Known broken tasks" note for runBenchmarkCompose/View/All -- this PR fixes those tasks, so the note now contradicted the "Resolved" section later in the same file. - BenchmarkUtils.kt: update the class KDoc, which still said "minimal and free of instrumentation-specific side-effects" despite now containing Logcat capability-report emission and instrumentation-dependent helpers. * docs: add NEXT_STEPS.md consolidating remaining work Pulls together what's left after this PR's audit pass into one prioritized, actionable list (P0: needs a device/emulator run; P1: safe code fixes; P2: research-quality polish; P3: minor). Matches the NEXT_STEPS.md convention already used elsewhere in this workspace (see JMW/game/NEXT_STEPS.md). * docs: add AI-assisted code review for Compose vs Android View performance project - Architecture review (4 modules, ~2,500 LOC) - Module-by-module findings and recommendations
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-on maintenance pass after PR #25 (merged). Fixes several issues flagged in
CLAUDE.md's audit findings plus a few more found along the way.Changes
runBenchmarkCompose/runBenchmarkView/runAllBenchmarksGradle task graph (referenced task paths that didn't exist; verified via--dry-run)MetricCapabilityarchitecture intocoldStartup_compose()/coldStartup_view()— moved capability-report logic out of a dead, never-compiledandroidTestfile (which also had a wrong import) into the realsrc/main/BenchmarkUtils.ktresults/README.mdpolicyREADME.md/manifest cited "2026-08-07," but raw trace timestamps show the run happened 2026-07-06; numbers unchanged, only the date/filenames were wronglocal.propertiesSDK-path claimMainActivityfromAppCompatActivitytoComponentActivityfor parity withapp-compose(tracked as TODO feat(ANDPERF-3): add benchmark module setup for Compose vs View apps #6) — a real confound in a study whose core claim is a fair cold-start comparison. Not yet re-benchmarked on a device, so published cold-start numbers may no longer be accurate.REVIEW.mdin this checkout contains at least one incorrect recommendation (DiffUtil on a static, never-updated adapter)Verification
./gradlew build(all modules, all variants, unit tests) passes./gradlew runBenchmarkCompose runBenchmarkView runAllBenchmarks --dry-runresolves cleanly:shared:testDebugUnitTest(MetricCapability unit tests) passesassembleBenchmark/assembleRelease(incl.lintVitalRelease) pass forapp-viewafter theComponentActivityswitchCLAUDE.mdfor what still needs a real device/emulator.Test plan
ComponentActivitychange doesn't regress and to refresh the published numberscoldStartup_compose/coldStartup_viewcapability-report logging (adb logcat -s BenchmarkCapability) on a real device