Skip to content

perf(harmonyos): memoize gesture viewport dumps - #2051

Merged
thymikee merged 3 commits into
mainfrom
perf/a11-leftovers-batch
Aug 26, 2026
Merged

perf(harmonyos): memoize gesture viewport dumps#2051
thymikee merged 3 commits into
mainfrom
perf/a11-leftovers-batch

Conversation

@thymikee

@thymikee thymikee commented Aug 26, 2026

Copy link
Copy Markdown
Member

Summary

HarmonyOS coordinate gestures now reuse one device-scoped ArkUI viewport for up to two seconds instead of paying a full layout dump for every adjacent gesture. Foreground-changing actions invalidate the memo, and keyboard dismiss delegates Back through the same invalidating owner as normal navigation.

The previous diagnostics and Apple changes were removed from this PR and preserved as separate ownership slices. This PR changes 7 files and stays within the HarmonyOS interaction family.

Validation

A planted 20-second TTL made the new exact-expiry regression return the stale portrait viewport after two seconds. Before the production fix, the real interactor keyboard-dismiss route emitted Back but did not invalidate the memo. Both regressions are green on c604df249e.

The affected gate passed all runnable checks, including 247 test files / 1,550 tests, format, lint, typecheck, layering, Fallow, build, and provider integration. No HarmonyOS hardware is provisioned locally or in CI; residual live risk is limited to physical rotation or multi-window geometry changes within the two-second TTL.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed current head 6fba42957: not ready. src/utils/diagnostics.ts adds a process-global, never-cleared ensuredDiagnosticDirs set. A long-lived daemon retains every distinct debug/trace directory forever; worse, if a previously seen directory is removed, later scopes skip mkdirSync, the append fails, and emitDiagnostic swallows it—silently losing diagnostics. Keep this state request/scope-local (or remove the cache) and add a two-scope delete/recreate regression. The branch is also DIRTY/conflicting against current main, and only CodeQL ran on this head, so rebase and rerun full CI. HarmonyOS hardware remains an explicit residual evidence gap.

@thymikee
thymikee force-pushed the perf/a11-leftovers-batch branch from 6fba429 to 9a58fc4 Compare August 26, 2026 14:32
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto current main and reconciled with #2050's package-owned Apple runner. The diagnostics directory memo is now scope-owned, including a delete/recreate regression; the stale-bundle test now lives under packages/platform-apple. Full local affected gate passed on 9a58fc4.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 26, 2026
@github-actions

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.45 MB 2.45 MB +533 B
JS gzip 821.1 kB 821.4 kB +217 B
npm tarball 943.9 kB 944.2 kB +267 B
npm unpacked 3.28 MB 3.28 MB +533 B

npm unpacked components

Component Base Current Diff
JS / dist source 2.61 MB 2.61 MB +533 B
Apple runner source/project 570.8 kB 570.8 kB 0 B
macOS helper source 54.8 kB 54.8 kB 0 B
Android helper artifacts 0 B 0 B 0 B
Other package files 45.3 kB 45.3 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 34.8 ms 34.8 ms +0.1 ms
CLI --help 93.5 ms 90.0 ms -3.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/runner-client.js +25 B -6 B

Top changed packed files

Packed file Base Current Diff
dist/src/app-lifecycle2.js 2.8 kB 5.3 kB +2.5 kB
dist/src/harmonyos.js 6.6 kB 4.4 kB -2.2 kB
dist/src/sdk-android-adb.js 21.7 kB 22.0 kB +263 B
dist/src/apps.js 22.8 kB 22.8 kB -32 B
dist/src/runner-client.js 85.9 kB 85.9 kB +25 B
dist/src/simulator.js 3.2 kB 3.2 kB -7 B

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 26, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Two P2 readiness blockers remain:

  1. Add planted-red TTL-expiry coverage for the Harmony viewport cache. The new tests prove reuse, explicit invalidation, and device scoping, but never cross the configured 2,000ms TTL. A wrong or non-expiring TTL would stay green. Use a fake clock/timer to cache a viewport, cross 2s without an invalidation hook, and prove the next lookup performs a fresh dump.

  2. Split this four-family leftovers batch into independently reviewable ownership slices: diagnostics scope/redaction, package-Apple stale-bundle cleanup, iOS boot memo reuse, and Harmony viewport caching share no owning interface or validation/evidence path. The refactor(apple): colocate the XCUITest runner client into packages/platform-apple (#2040) #2050 rebase explains the Apple test location but not a runtime dependency. This conflicts with the repository rule to keep changes within one command family/module group unless the task explicitly crosses a boundary.

[P1] Invalidate the Harmony viewport memo when pressHarmonyKeyboardKey receives Back. The Harmony interactor implements keyboard dismiss through pressHarmonyKeyboardKey(device, 'Back'), but this path emits HDC Back without the invalidation performed by backHarmony. If Back navigates (or no keyboard consumes it), the next gesture can use stale geometry for 2s. Add a regression through the keyboard-dismiss/interactor route.

The individual implementations otherwise look sound. Exact-head CI is still incomplete, and Harmony hardware evidence remains unavailable.

readHarmonyGestureViewport paid a full uitest dumpLayout + file recv + rm
per call, and edge scrolls read it once per pass. Cache the Application
rect per device id for 2s with eager (scheduled) expiry so long-lived
daemons do not leak entries, and invalidate after open/close app, home,
back, and app switcher - the foreground transitions agent-device itself
drives. Rotation or split-screen changes inside the window that no hook
covers yield one stale plan until expiry; documented at the memo.
Assert that backHarmony, homeHarmony, and appSwitcherHarmony call
invalidateHarmonyGestureViewport, and that openHarmonyApp/closeHarmonyApp
do so on their success paths, so deleting the hook call sites can no
longer leave the suite green.
@thymikee
thymikee force-pushed the perf/a11-leftovers-batch branch from 9a58fc4 to c604df2 Compare August 26, 2026 15:09
@thymikee thymikee changed the title perf: A11 leftovers — diagnostics flush cost, parallel stale-bundle cleanup, booted-memo reuse, HarmonyOS viewport cache perf(harmonyos): memoize gesture viewport dumps Aug 26, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Re-review summary at c604df249ee51daca15b238aa11607c90e864cc3:

No remaining code-review finding is known on this head. CI is still authoritative for the final merge verdict.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 26, 2026
@thymikee
thymikee merged commit adcf263 into main Aug 26, 2026
1 of 2 checks passed
@thymikee
thymikee deleted the perf/a11-leftovers-batch branch August 26, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant