mirror of
https://github.com/Gathin23/cleanyourtv.git
synced 2026-11-07 21:26:41 +00:00
docs/workflows/perf-ab.md: a regression check for every launcher change
The last piece proposed in #564, and a suggestion for the maintainer to accept or decline: every launcher change starts with a tools/bench.py baseline and shows it doesn't make the launcher slower or heavier. - perf-ab.md opens with the regression check: bench.py on the base build before changing anything, again on the change, and six result.json fields with limits (key response, return to Home, cold start, jank, PSS, idle CPU). A failing field sends the agent to the trace and flame chart method below it to find the cost, fix it and measure again. - The limits sit above the noise of two runs of the same build on a TCL C755 (0 ms key response, 45 ms cold start, 1 MB PSS). A full run takes about 6 minutes since #949. - AGENTS.md's switch recipe gains the check as step 9, and its PR checklist a Performance row. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
7de6947bef
commit
2fd20fbda4
2 files changed
+48
-5
No files matched your search
@@ -174,6 +174,8 @@ The core takes a switch only if it changes nothing until a theme or `config.xml`
|
||||
7. Keep it small: a switch should cost a few hundred bytes to about 1 KB of APK. Build and check the size
|
||||
gate; the gate is hard.
|
||||
8. Verify on a real TV with the switch on and off, D-pad from every edge, and attach screenshots.
|
||||
9. Run the regression check in [`docs/workflows/perf-ab.md`](docs/workflows/perf-ab.md) with the switch on: a
|
||||
baseline before you start, your build after, and the numbers in the PR.
|
||||
|
||||
Anything TV-specific goes in `config.xml` or a theme, not in code. A bug fix that corrects wrong behaviour needs
|
||||
no switch. No new dependencies in `launcher/app/build.gradle` without an issue first.
|
||||
@@ -239,8 +241,8 @@ no switch. No new dependencies in `launcher/app/build.gradle` without an issue f
|
||||
Multi-step methods in `docs/workflows/`. Read the one that fits before you start; each opens with when to
|
||||
use it and links the tool guides it needs.
|
||||
|
||||
- [`perf-ab.md`](docs/workflows/perf-ab.md): measure performance improvements or regressions from a launcher
|
||||
change, with traces, a report and flame charts.
|
||||
- [`perf-ab.md`](docs/workflows/perf-ab.md): check every launcher change for performance regressions with
|
||||
`tools/bench.py`, and measure an improvement or a failed check with traces, a report and flame charts.
|
||||
|
||||
## Before opening a PR
|
||||
|
||||
@@ -253,7 +255,8 @@ use it and links the tool guides it needs.
|
||||
| One topic per PR | A TV list never rides inside a launcher feature PR. A device's list plus small fixes for that same device can be one PR. |
|
||||
| Tested on | Say which TV, Android version and firmware you tested on, in the PR template's "Tested on" line. |
|
||||
| Real TV | Launcher changes are verified on a real TV, not only the emulator: `tools/tv.py --screenshot shots/after.png DPAD_DOWN`. D-pad from every edge (Up, Down, Left, Right). A button handled by an accessibility helper needs a press on the real remote, and a force-stop of the launcher switches the helpers off (`docs/TROUBLESHOOTING.md`). |
|
||||
| Screenshots | Every visual change has a screenshot from the TV in the PR description. A performance claim has numbers, for example from `tools/bench.py`. |
|
||||
| Screenshots | Every visual change has a screenshot from the TV in the PR description. |
|
||||
| Performance | A launcher change shows `tools/bench.py` before and after and passes the regression check in [`docs/workflows/perf-ab.md`](docs/workflows/perf-ab.md); a performance claim adds trace numbers. |
|
||||
| Stray files | No files that nothing links to: screenshots belong in the PR description, not the tree. |
|
||||
|
||||
## Hard rules
|
||||
|
||||
@@ -1,7 +1,47 @@
|
||||
# Workflow: measure a launcher performance change, before and after
|
||||
|
||||
**When to use this:** you changed the launcher to make it faster or lighter, and the pull request has to
|
||||
show by how much, and where the time went. This is the method #666 used.
|
||||
**When to use this:** any change under `launcher/`. Every one runs the regression check below. A change
|
||||
meant to make the launcher faster or lighter, or one whose check fails, then follows the full method
|
||||
#666 used, to show by how much and where the time went.
|
||||
|
||||
## The regression check, for every launcher change
|
||||
|
||||
1. **Before you change anything,** install the base build with `launcher/build-and-install.sh` and run:
|
||||
|
||||
```bash
|
||||
TV_HOST=<tv ip> tools/venv/bin/python tools/bench.py --out-dir work/bench-before
|
||||
```
|
||||
|
||||
2. Make the change, install your build the same way, and run `tools/bench.py --out-dir work/bench-after`.
|
||||
3. Compare the two `result.json` files. Leave out any field that is `null` in both:
|
||||
|
||||
| Field | Fails when the after value is |
|
||||
|---|---|
|
||||
| `response_ms.p50` | more than 10% and 5 ms above before |
|
||||
| `return_to_home_ms.p50` | more than 10% and 10 ms above before |
|
||||
| `cold_start_ms.p50` | more than 10% and 30 ms above before |
|
||||
| `jank_pct` | more than 1 point above before |
|
||||
| `pss_mb` | more than 2 MB above before |
|
||||
| `idle_launcher_cpu_pct` | more than 0.5 points above before |
|
||||
|
||||
4. **If a field fails,** follow the method below with that scenario (`home`, `cold` or `nav`) to find
|
||||
the cost. Fix it and go back to step 2. Fixes that usually work:
|
||||
- doing the work after the first frame;
|
||||
- skipping work when nothing changed;
|
||||
- decoding images at their display size;
|
||||
- keeping results instead of recomputing them.
|
||||
5. Put the six fields, before and after, in the PR. If a feature has to cost something, give the number
|
||||
and the reason. Once it works, spend one more pass on its cost.
|
||||
|
||||
Some notes on the numbers:
|
||||
- **Noise:** two runs of the same build on a TCL C755 differed by 0 ms in key response, up to 45 ms in
|
||||
cold start and 1 MB in PSS, and the limits sit above that. Rerun both builds if a field lands near
|
||||
its limit.
|
||||
- **Run time:** a full run takes about 6 minutes.
|
||||
- **TVs that don't log injected keys:** `return_to_home_ms` is `null`, and `response_ms` includes the ADB
|
||||
round trip. It still compares two builds on the same TV.
|
||||
|
||||
## The full method
|
||||
|
||||
Tools: [`tools/trace.py` and `tools/trace_report.py`](../../tools/trace.md),
|
||||
[`tools/flamechart.py`](../../tools/flamechart.md) and [`tools/screendiff.py`](../../tools/screendiff.md).
|
||||
|
||||
Reference in new issue
Block a user