refactor: split Android native perf collector#788
Merged
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
afc3190 to
c7505ca
Compare
Member
Author
|
I reran the Android live smoke from an isolated review worktree because the PR body currently says emulator validation was blocked. Validation passed on a real headless emulator:
Please update the PR body to replace the blocked-live-smoke note with this evidence before merge. I am continuing the code review separately. |
Member
Author
|
CI has one real blocker:
This looks like a simple refactor cleanup: keep |
Member
Author
|
I pushed the Fallow cleanup as
CI should rerun on the new head. |
|
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
Split Android native perf from one 741-line collector into focused modules for process/tool resolution, artifact lifecycle, error hint classification, Simpleperf, Perfetto, compact summary shaping, and shared native perf types. The public perf-native.ts surface remains a small facade so existing callers keep the same imports.
Rebased onto latest main after #772. The branch does not touch command-surface files; the merged #772 layout keeps perf/debug/log/network under src/commands/observability, with debug symbols in the same family. Focused command-family tests confirm this PR is compatible with that shape.
Closes #785.
Touched files: 8.
Validation
Focused Android perf/session and command-family coverage passed after rebase: pnpm exec vitest run src/platforms/android/tests/perf.test.ts src/daemon/tests/request-router-android-perf.test.ts src/daemon/handlers/tests/session-observability.test.ts src/commands/observability/index.test.ts src/commands/tests/command-surface-metadata.test.ts src/utils/tests/args.test.ts (6 files, 162 tests). pnpm check:quick passed lint and typecheck. pnpm build passed before the rebase. git diff --check passed after rebase.
Before/after evidence: before, src/platforms/android/perf-native.ts mixed native process resolution, collectors, artifact handling, hints, and summary shaping in one 741-line file. After, the facade is 18 lines and each focused Android native perf module is <=215 LOC while preserving compact artifact-path output and unavailable hints.
pnpm check:unit was rerun twice before the rebase and still failed outside this change area in mocked install/runtime/device/screenshot tests, e.g. src/platforms/tests/install-source.test.ts, src/platforms/android/tests/devices.test.ts, src/platforms/android/tests/index.test.ts, and src/platforms/ios/tests/index.test.ts.
Live emulator smoke was blocked: node bin/agent-device.mjs devices --platform android --json returned an empty device list, and apps --platform android returned DEVICE_NOT_FOUND. Positive Simpleperf capture still requires a booted Android emulator/device, an opened debuggable/profileable app, and a device image that permits perf_event_open; otherwise the expected result is the explicit unavailable/actionable hint path.