5a6ae3f71dd177d58c03d5f0260c9efc3f62befd
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
5a6ae3f71d |
Record what actually blocks the interaction benchmarks: the reinstall clears the grant
The diagnosis is now precise and it is not the installer dialog. Macrobenchmark's rule uninstalls the app and reinstalls it before measuring; an uninstall clears app-ops, so the MANAGE_EXTERNAL_STORAGE grant that tools/benchmark-fixture.sh applies is gone by the time the test navigates. appops get reads No operations after a run, the app correctly shows its choose-a-folder fallback, and the volume row the test looks for is not on screen - which is exactly the assertion that fired. No test can work around it: granting an app-op needs the shell uid, and UiAutomation.executeShellCommand runs as the test app. So the three interaction benchmarks cannot run as macrobenchmark tests on any device where this app needs the grant. PERFORMANCE.md now says that, names the two paths that would give the numbers (a plain instrumented test that avoids the rule, or a benchmark flow that needs no grant), and says what each costs. The cold-start number stands. |
||
|
|
cb2db9ab63 |
Make the benchmark build the app itself, and record what the runner does to it
A separate package id for the app under test is the tidier design and cannot be used on this device: a new package id needs a human to approve the install every time, and installing by tapping the APK left the package listed by pm list packages yet unknown to pm enable and invisible to PackageManager. The benchmark build type therefore keeps the app's own id and takes the release key, so it updates an installed release build and keeps its data. The trade-off is stated where it happens: this build replaces a release install, and reinstalling the release APK puts it back. Two bugs surfaced on the way and both are now commented at the line they belong to. -PbenchmarkBuild was read with providers.gradleProperty, which reported it absent, so the runner installed the debug APK and failed on it; hasProperty is what works. And the reason am instrument cannot be used for this module is now recorded: MacrobenchmarkRule uninstalls the target at the start of a run and reinstalls it from the APK paths the Gradle runner passes in, so without those the app is uninstalled and left that way - which is what produced NameNotFoundException for an app that was installed a second earlier, twice, with Benchmark: Uninstalling in logcat to prove it. coldStart keeps its measured number (532 ms median). The three interaction benchmarks remain unmeasured for a reason that is now written down: each run needs one tap on the phone for the reinstall MIUI gates. Gate green, map current. |
||
|
|
94f85bd287 |
Get a real cold-start number out of the benchmark, and record what the device demanded
coldStart measures 532 ms median (min 499, max 805) on the benchmark build - R8, not debuggable, profileable, CompilationMode.None - against 1814 ms on the debug build, which is the honest illustration of why a debug number must never be published as the app's performance. The tool also warns that it cannot clear the runtime image on this Android version, so the figure is optimistic by an unmeasured amount; that is in PERFORMANCE.md next to the number rather than in a footnote. The three interaction benchmarks are written and compile but have not produced a number, and getting that far took four fixes worth naming. The app under test cannot keep the app's package id (signature) and cannot use .benchmark (the test APK owns that id, INSTALL_FAILED_VERSION_DOWNGRADE); macrobenchmark needs package visibility for it or it reports NameNotFoundException for an app that pm list packages shows installed; UiAutomation.executeShellCommand runs as the test app's uid, so the fixture has to be made through adb instead - that one cost the first attempt a hang; and testBuildType has to be behind a flag or :app:testDebugUnitTest stops existing. The workflow that works on a device with an install gate is now documented: install the two APKs by hand once, run tools/benchmark-fixture.sh, then am instrument. The Gradle task is documented as the alternative and as the reason the app under test disappeared once - it uninstalls around the run. Gate green, map current, 386 tests unchanged. |
||
|
|
1f94d0a38a |
Add a macrobenchmark module, and find out what the build was hiding
docs/PERFORMANCE.md has been honest that three of its four numbers do not exist. They exist now as measurements, if not yet as results: benchmark/ is a macrobenchmark module with four tests - cold start with StartupTimingMetric, then opening, scrolling and searching a folder of a thousand files with FrameTimingMetric - sharing a fixture at /sdcard/AABenchmark that is created once per run and deleted in @AfterClass. Frames rather than a named trace section on purpose: what a user calls slow is jank on screen, and the alternative puts benchmarking calls into production code for the sake of a number. CompilationMode.None() is the honest cold-install baseline; Partial() plus a baseline profile is the follow-up and needs the same module. The storage grant is applied with appops in setup, which is what lets the run be unattended - a permission dialog is the one thing a test cannot answer. Three build facts had to be discovered rather than designed, and each is now written where it will be read. The app under test cannot keep its own package id: it needs a debug signature to install on a test device, so it cannot replace a release install, and an un-suffixed build is exactly that. It also cannot use the obvious suffix: `.benchmark` is already the benchmark module's own APK id, and the test APK then fails to install with INSTALL_FAILED_VERSION_DOWNGRADE - which is what the first attempt did. Hence `.perf`. And `testBuildType = "benchmark"` cannot be set unconditionally, because it makes benchmark the variant that *unit* tests run against and :app:testDebugUnitTest - the task the entire gate is built on - stops existing; it is behind -PbenchmarkBuild instead. All three were measured, not guessed, and the third would have broken CI silently. Two smaller things came with it. tools/verify.sh --release now also compiles the app's benchmark variant and the test APK, because build wiring nobody runs is wiring that rots; no device is involved. And the code map grew an EXTRA_ROOTS section so it indexes benchmark/ too - a map that silently omits a module lies by omission, which is worse than the one line it costs. The numbers themselves are still missing, and for a device reason rather than a code one: installing the new package id on this MIUI phone fails with INSTALL_FAILED_USER_RESTRICTED (the "Install via USB" gate, which wants a human and answered too late). Both APKs are pushed to /sdcard/Download/ for a manual install, and the run is one command away: `adb shell am instrument -w -e class de.softwareapp_hb.filemanager.benchmark.BrowserBenchmark de.softwareapp_hb.filemanager.benchmark/androidx.test.runner.AndroidJUnitRunner`. Gate green with --release, including the new compile step; map current; 386 tests unchanged. |
||
|
|
0bbee25ab3 |
Test the interface as a user sees it, and let a debug build tell on itself
Fifteen rendering tests across four files are the first tests in this project that compose a screen, and they cover the claims that used to be prose only: the app bar's star exists where there is a folder to star and says Add or Remove according to the state, the selection bar offers Rename to several files and Details only to one, the batch dialog spells out the names it will produce and refuses a collision with the file named, the shared name dialog refuses a slash while it is typed, and a favourite that has gone says "Folder not found" rather than showing a path that is not there. They run on the JVM through Robolectric - the Compose test artifacts plus the test manifest - so they belong to the same verify.sh gate as everything else instead of needing a device: 386 tests in total. Writing them turned up one behaviour worth recording. The batch dialog continues the folder's series by default, so a collision cannot be shown unless the start number is forced back to 1 - which is precisely why the feature works in practice, and now also why two of these tests set the field explicitly. The debug build now enables StrictMode (disk reads and writes, network, leaked closables; logged, not fatal) and attaches LeakCanary. A file manager is the worst possible app for a blocking disk read on the main thread, and a debug build is the last place where noticing it is still cheap. Logging rather than a crash is deliberate: turning a slow screen into a crash on someone's phone is the worse trade. The flag is read from the manifest, not from BuildConfig, which this build does not generate. Performance has a number for the first time, and it immediately paid for itself. tools/measure-startup.sh takes cold start with `am start -W` - median of several runs, one warm-up discarded because the first launch after an install pays for dexopt - and docs/PERFORMANCE.md records 1814 ms over five runs, in the debug build, which is the slow case by construction. StrictMode's first run then produced five violations against that number: 492 ms resolving `app.cacheDir` inside the view-model factory, 397 ms reading free space with StatFs for every volume inside refresh(), 477 ms of the platform loading its own typefaces, and two 54 ms OEM font hooks. That is about 1.5 s of main-thread disk I/O in a 1.8 s start, of which ~0.9 s is ours and mechanical to move. Neither of ours is fixed here on purpose: the numbers and the two fixes are recorded in PERFORMANCE.md, and they are the first items of the speed work. What is not measured is stated in the same file rather than left to be assumed: folder open at 1 000 / 5 000 / 20 000 files, thumbnail paint while scrolling, and keystroke-to-result all need a `:benchmark` module and a profileable build under the release package id, and this device waits for a confirmation dialog on the first install of a new package id. The module is deliberately absent instead of half-configured: build wiring nobody can run is worse than a recorded gap. Gate green (map current, detekt, 386 tests, lint), and CHANGELOG, README, AGENTS.md, ROADMAP, PERFORMANCE.md, EDGE_CASE_TESTING and the regenerated map carry it. |