diff --git a/AGENTS.md b/AGENTS.md index 48ff99573..8fe536e63 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/docs/workflows/perf-ab.md b/docs/workflows/perf-ab.md index bab51c9fa..eb6bf013e 100644 --- a/docs/workflows/perf-ab.md +++ b/docs/workflows/perf-ab.md @@ -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= 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).