Repository navigation
perf(android): reuse live helper for input and capture - #3811
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
Three problems remain at ddba75f, so this PR needs changes before merge. Each one turns a call that adb completes today into a failure when the live helper session is active. Fill now fails before any adb attempt when the helper refuses. With a live helper session, Screenshot follows the mutation policy by mistake. The live text read can now return stale text for list rows. Would a smaller shape work? Keep the session channel and put one action-trait table in Not blocking, take or leave: the help and website say I read the code at ddba75f. The emulator benchmark and the full test-app suite run are taken from the docs and evidence in the PR, and I did not reproduce them. Those live runs cover a Pixel 7 API 36 emulator only, so a physical device, API below 33, the helper-unavailable fallback, and a fill on a field that is not editable at the point are not covered. I did not check whether Smoke Tests was still running when I checked, with no failure to attribute yet. This PR changes default screenshot behavior and CLI help text, which smoke runs may exercise. There are no conflicts. Before merge, helper failures must follow ADR 0002 through one action-trait table, with screenshot and focus falling back to adb and a fill refused before any injection falling back too, and the identity text read must pick by point among duplicate ids, each with a regression test. |
|
The head delta since ddba75f only adds evidence docs, so the three defects from the earlier review are still in the code at 0d4b58c. Nothing from that review is fixed yet. The 42-case parity run on a Pixel 7 API 36 emulator covers the happy path only, and I did not reproduce it. First, fillAndroid sends Second, Third, readAndroidTextAtPointInHierarchy ignores (x,y) when My simplicity question from the earlier review is still open, since the delta did not touch it. Could the session channel stay, with one action-trait table in After the first two fixes, please run live on Android with a helper session active and show two cases. A fill on a field the helper refuses (no editable node at the point) must complete through adb with the expected text, and the daemon log must show the refusal fallback diagnostic. A forced screenshot helper failure must produce a valid PNG through I did not test on a physical device, on API below 33, through the helper-unavailable fallback, or on Appium Android. I also did not compare Smoke Tests was still running when I checked, and no check has failed. Before merge, the three fixes above need regression tests that fail on head, and the live run above needs to pass. |
|
Addressed the three blocking findings in runtime
The new 13-case regression suite on original Live Pixel 7/API 36, rebuilt helper
For the optional points: help/docs now say ADB-backed Android. The action policy is centralized. I retained the existing named key operations, the base-reproduced TV fixture correction, and the moved-text fix; splitting them further would add stack churn without changing these verified contracts. Duplicate focus dumps and diagnostic-phase renaming remain outside this correctness round. Physical/older-API devices, Appium, and actual-head CI remain unverified here. No new whole-suite performance result is claimed. All owned sessions/daemons were closed. These reviews were conversation comments; there were no inline threads to resolve. Current-main conflict follow-up: rebased onto |
2f85c9c to
1977e68
Compare
|
Checked latest Fresh Android comparison: pre-daemon stack Observed p50 in milliseconds:
No median regression reached 100 ms in these workloads. Every warm package open used zero host Cold follow-up: 20 fresh-daemon/app-start cycles, alternating source order, force-stopped app, identical source-built native APK installed before timing. Open median 2,220→2,054 ms; close 277→261 ms; daemon stop 187→187 ms. Both sources used two helper starts during cold foreground open; the extra start observed in one primary cold trial is also present on the older implementation. Tails remain visible: one primary moving-clear trial took 791 ms, with additional reads in the existing input/verification path, and one cold open took 3.32 s. There was no fallback, resend or wrong edit. The fill verifier and native Java input source are unchanged. A dedicated alternating repeat added 16 moving-clear trials: before median 276.5 ms, range 257–327; current median 279 ms, range 262–290. All had one clear and zero decoy edits/clicks. This is a small local regression check, not a statistical tail-latency guarantee or a new full 42-case speedup result. Scroll-until needed one or two passes depending on observed content position. The initial harness attempt failed its scroll target after generic Back; it is retained and excluded. Corrected runs use explicit keyboard dismissal and verify target visibility. All owned sessions/daemons closed. Raw samples, traces, scripts and the excluded setup run are retained locally. No code/push was needed; these measurements do not replace actual-head CI obligations. |
|
The PR is ready. The fixes from the earlier review (#3811 (comment)) are in at 1977e68, and I found no new problems in the code. Not blocking: the cancellable-input check ( Smoke Tests was still running when I checked and had no failed log. This diff changes the Android helper APK and the input, screenshot and fill routes, so the Android smoke lane should exercise it. I can't attribute the CI result until it finishes. No conflicts. On evidence, the live Pixel 7 (API 36) refusal-fallback and screenshot-fallback runs appear only in your comment from 2026-10-19T15:50Z, with no daemon-log excerpt, and I did not reproduce them. I did not run the regression suite, and I judged that the new tests fail on 0d4b58c by reading the old code. I did not check physical devices, API levels below 33, Appium Android, or screenshot orientation of UiAutomation.takeScreenshot versus screencap on rotated displays. Smoke Tests needs to finish green on 1977e68 before merge. |
|
Fresh full Android benchmark on the final combined stack, using the original experiment’s unchanged app and test sources.
Current saves 61.080 seconds (18.2% less elapsed time) versus the original baseline, and 8.011 seconds (2.8%) versus the pre-daemon stack. The daemon/main composition did not regress this full run. All three reports contain exactly the same 43 selected identities / 42 runnable identities, matched against the reference case registry. Every runnable test passed in one attempt. Every attempt reports complete cleanup and no secondary errors. Runner retries were disabled; the fixture’s authored second-tap recovery at This measures #3811 + #3815 + the readiness opt-in from e2e #1253, including intervening main changes. It is not a standalone speedup attribution to either SDK PR or daemon work. The adapter is pinned at Method: Pixel_7_CI / emulator-5554, Android 16/API 36; one worker; zero retries; Device panel stream attached throughout. Release APK SHA-256 Largest original→current case reductions:
Current vs pre-daemon is mixed by case: bottom tabs took 1.023 s longer and error recovery 0.975 s longer; the total still improved by 8.011 s. This is one run per arm with host-load variation, not a statistical guarantee of the 2.8% difference or tail latency. The experimental direct engine, physical devices and older Android APIs were not rerun. Exact matched runnable case IDs and attempt durations (seconds)
All owned sessions and daemons closed gracefully. Source SHAs and clean worktree state remained unchanged. Local artifacts retain full reports, source/dependency provenance, runner logs, setup snapshots and analysis scripts. Actual-head CI obligations remain separate. |
Summary
Reuse the live Android helper for input, verified fill, focus, and PNG capture. Uncertain mutations fail without a resend. Validated no-injection refusals and failed reads stop the helper before using the host path. Dead sessions retire before piggyback commands. Duplicate resource IDs retain live point reads within the requested package/class.
Historical original-app measurements: tap p50 120→67.7 ms, fill 972→760 ms, screenshot 1448→311 ms; 42/42 cases on both variants. Cases and method. These precede the review fixes.
55 files: production +620/−181; tests +670/−5; docs +994/−4; 2,474 gross lines. Scope includes the evidenced live-text and Android TV fixture fixes. Reference: e2e#1234. Follow-up: #3815; adapter #1253 needs an SDK release.
Validation
Final
1977e685b2, rebased ontomaina661e43d83: Affected gate passed: 16,388 tests passed, one skipped; receiptrun-gWuy8f,exactCommit=true,checkoutChanged=false. Cancellable tap/back calls retain the upstream host path; ordinary input retains helper reuse. Independent rebase review passed.New regressions: original
0d4b58c3418 failed/5 passed; fixed runtime all 13 passed. Two live-helper signal-forwarding cases and 56 focused input/alert tests passed. Live API 36: real fill refusal→ADB success and controlled screenshot protocol failure→valid screencap PNG, with both fallback diagnostics. Independent reviews passed after corrections.Physical devices, older APIs, Appium, and actual-head CI coverage/native/device/provider lanes remain separate obligations.
Fresh combined-stack benchmark: original 335.069→current 273.989 s (18.2% reduction); pre-daemon 282.000 s. All 42 passed, one authored skip, zero retries/recoveries. One local run per arm.