diff options
| -rw-r--r-- | .fastfeedback/SPEED.md | 203 |
1 files changed, 203 insertions, 0 deletions
diff --git a/.fastfeedback/SPEED.md b/.fastfeedback/SPEED.md new file mode 100644 index 0000000..8dbe860 --- /dev/null +++ b/.fastfeedback/SPEED.md @@ -0,0 +1,203 @@ +# SPEED — ida-tui-maybe + +_Read this BEFORE running anything slow._ This repo does **not** use pytest for its +suites; every suite is a standalone script with its own runner and its own tally line +(`N passed, M failed` / `N checks, M failed`). `speedscan`'s generic pytest template is +wrong here — these are the real commands. + +--- + +## The two interpreters + +| python | has | use for | +|---|---|---| +| `python3` (system) | stdlib only | every **pure** suite. No IDA, no Code Mode. | +| `~/ida-venv/bin/python` | textual + idapro + ida_codemode | anything marked `ida`, and the TUI itself | + +`tests/run.py --list` prints `pure` / `ida` per file. A `pure` file must keep running +under system `python3` — that is a house rule, and it is why `idatui/codemode_client.py` +defers its `ida_codemode` import instead of doing it at module top. + +--- + +## Escalation ladder for THIS repo + +**Cheap gates (seconds, run these first)** +```bash +python3 -c "import ast;ast.parse(open('idatui/codemode_client.py').read())" # ~0.05s syntax +python3 tests/run.py --fast # every pure suite, 302 checks, 0.6s +python3 tests/run.py graph -x # one suite by substring, fail-fast +``` + +**Narrow test — DEFAULT RUNG.** The pilot suite takes `--only <substr,...>`: +```bash +~/ida-venv/bin/python tests/test_scenarios.py /tmp/scratch_bin --only rename # ~10s +~/ida-venv/bin/python tests/test_scenarios.py --list # scenario names +``` +`--only` matches substrings, so `--only listing` runs listing_view + listing_name_addr + +listing_make_string + listing_struct_expand. Name the scenario exactly to stay narrow. + +**Full gate — BACKGROUND ONLY, once, at the end** (see next section for the launch form): +```bash +~/ida-venv/bin/python tests/run.py # every suite, serial on purpose +``` + +--- + +## Coverage map — which suite catches what + +The pilot suite is big and slow and it is **not** the whole story. Two backend bugs +this port shipped were invisible to `test_scenarios.py` and only fell out of the +smaller RPC/UI suites: + +| suite | ~time | catches | +|---|---|---| +| `test_scenarios.py` | 166s | the TUI end to end: nav, views, listing, graph, opfmt, rename-one | +| `test_rawimage_rpc.py` | ~60s | **`rename_many` (batch rename), define/blob workflow, opfmt over RPC** | +| `test_blob_ui.py` | ~40s | raw-image/blob UI | +| `test_project_ui.py` | ~8s | multi-binary project UI | +| `test_trace_rpc.py` / `test_trace_ui.py` | ~30s | Tenet trace integration | + +**If you touch a backend operation, grep for its callers and run the suite that owns +them — not just the pilot.** `rename` is exercised one-edit-at-a-time by the pilot and +as a *batch* only by `test_rawimage_rpc.py`, which is exactly where the list-vs-dict +bug hid. + +--- + +## Backgrounding in THIS harness — the gotcha that cost a turn + +`bgrun run ... ` **blocks the agent tool call** even though bgrun itself returns in ms. +`bgrun` line 64 is `( "$@" >"$LOG" 2>&1; echo $? >"$EXIT" ) &` — the command's own stdio +goes to the log, but the *backgrounded subshell* still holds the inherited stdout fd, and +the harness's bash waits for EOF on that pipe, not for the parent to exit. + +**Always launch detached with all three fds redirected:** +```bash +SK="$HOME/.the assistant/agent/skills/fast-feedback/scripts" +timeout 20 setsid "$SK/bgrun" run fullgate -- ~/ida-venv/bin/python tests/run.py \ + </dev/null >/tmp/bgrun.out 2>&1 ; cat /tmp/bgrun.out # returns in ~7ms +timeout 20 "$SK/bgrun" check fullgate # non-blocking, safe +``` +`bgrun check` / `bgrun list` do **not** need this treatment; only `run` does. + +**Never `pkill -f <pattern>` here.** The agent's own shell command line contains the +pattern you are matching, so `pkill -f "tests/run.py"` kills the very shell issuing it +and the rest of the command silently never runs. Kill by **pid** (`bgrun list`, +`pgrep -af` first), then gate on it with `waitfor pid-gone <pid>`. + +**Killing a pilot leaves a Code Mode worker behind** for its `--lease-grace` (20s); it +self-exits, and it is holding only that run's temp `.i64`, so it does not block a new +run. Do not `kill -9` it — a hard-killed IDA wedges its database. + +--- + +## The four ways a test here wastes minutes + +Every slow suite in this repo was slow for one of these, not for doing real work. +Check them before optimising anything else. + +1. **A wait on a signal that can no longer happen.** `wait(lambda: lst.model is not + old, ..., 60)` was the idiom for "the edit landed". The perf work made edits KEEP + the listing's walk and re-render in place, so the model object is never replaced: + every one of those waits sat out its full timeout and the check afterwards passed + **vacuously**. This alone was 30s in blob_ui and 4x60s in thumb_ui. + → Gate on what the check is about (the row is code, the status says Thumb, the + function is in the index), via `settle(app, pred)` from `idatui._sync`. +2. **A wait on a signal that is set too early.** `app._t` is assigned when the key is + handled; the navigation it starts runs in a worker. Waiting on it and then reading + the cursor is a race master won (single-digit ms backend) and Code Mode loses. + → `settle()` gates on quiescence *and* the predicate, which is what you want. +3. **Regenerated fixtures.** `os.urandom` into a fresh `TemporaryDirectory` means new + bytes at a new path every run, so the pristine-database cache can never apply and + full auto-analysis is paid forever. → `_fixtures.synthetic(name, build)` writes + deterministic bytes to a stable path; `staged()` then caches the analysis. + Determinism is also correctness: "this blob has no functions" must not depend on luck. +4. **Deleting a database and reopening the same path.** Safe when the TUI owned a + private worker; under Code Mode the previous phase's worker still holds the lease + for its grace period, so the delete races a live owner and the reopen yields no + listing. → Give each phase its own temp copy (`fresh_copy`). Never `os.remove` an + `.i64` a suite is about to reopen. + +**`settle(app, pred, timeout=...)`** (`idatui/_sync.py`) is the one true gate: it drains +the message pump, waits for workers, and returns the moment `pred` holds. It is what the +live RPC layer uses, so tests and driver agree on what "done" means. A bare +`settle(app)` (no pred) means "the app finished reacting" — the right gate when the edit +may legitimately do nothing (e.g. carving random bytes). + +## Measured timings (2026-08-07, this box, warm) + +| command | time | notes | +|---|---|---| +| `python3 tests/run.py --fast` | **0.6s** | 302 checks, all pure suites | +| `python3 tests/test_graph.py` | ~1s | 86 checks, pure layout engine | +| `python3 tests/test_codemode_client.py` | ~0.1s | 14 checks, fakes the DatabaseHandle | +| `~/ida-venv/bin/python tests/test_rawimage_rpc.py` | ~60s | 21 checks, owns batch rename | +| pilot boot (first scenario) | ~10-25s | opens the DB; seeded from `.pristine.i64` | +| pilot `--only graph` | ~10s | 50 checks | +| **pilot full (`test_scenarios.py`)** | **80.5s** | 56 scenarios, 301 checks (was 166s) | +| `test_trace_ui.py` | 22.0s | 39 checks | +| `test_rawimage_rpc.py` | 13.1s | 21 checks, owns batch rename | +| `test_project_ui.py` | 8.1s | 30 checks | +| `test_thumb_ui.py` | **8.5s** | 20 checks — was 313s AND crashing | +| `test_blob_ui.py` | **4.1s** | 30 checks — was 39.8s | +| `tests/run.py` (everything) | ~2m20s | was ~9m20s. BACKGROUND ONLY | + +IDA-suite total went from ~9m21s to ~2m16s (4.1x) by fixing the four wastes above — +not by removing a single check. + +### Backend performance note (why the pilot is 2.8x slower on codemode) +`heads` is the listing's paging call. Measured on `targets/echo`, 200 rows: + +| | master (worker: pickle over unix socket) | codemode (HTTP + JSON + `to_jsonable`) | +|---|---|---| +| `heads count=200` | 2.6 ms | 92 ms | +| `heads count=500` | 6.3 ms | 214 ms | +| empty round trip | — | 2.0 ms | + +So the transport floor is 2ms/call and the rest is per-row work + `to_jsonable` +(~71ms of a 200-row page). This is architectural, not a bug in the port. The +digest/`expect` path (a page that has not changed is answered with a hash and a count, +no rows) is the main mitigation and is implemented. + +**Corollary for testing:** the codemode backend is slow enough that UI settle races +appear that never appear on master. See KNOWN-FLAKY. + +--- + +## Known-flaky + +- `test_scenarios.py --only listing_view` (**codemode backend only**). Passed 2 runs, + failed the 3rd, on an unchanged tree and a fresh scratch DB. Symptom: `listing shows + data heads (not just code)` reports `{'label','sep','funchdr','code'}` and + `total=1500`, i.e. the model is read before its pages have materialised. Adding *any* + delay (even a single `stderr.write`) makes it pass, which is the signature of a settle + race, not a logic bug. The scenario waits on `c.lst.total > 0`, which becomes true + before the rows exist. **Do not chase this as a regression without reproducing it + 3+ times.** Real fix would be a wait on materialised rows, not on `total`. + +--- + +## Slow traps + +- **Never run the pilot suite against `targets/echo` directly.** It edits the database. + The suite already stages a scratch copy (`tests/_fixtures.py: staged()`), seeded from + `<binary>.pristine.i64` which nothing writes back to. Pass a **copy** in `/tmp` when + invoking by hand, or you are testing your own history. +- **`tests/run.py` runs suites serially on purpose.** Running them 4-up took the suite + from 153s to 296s and got three killed mid-analysis (idalib contends hard). Do not + "optimise" it with parallelism. +- **Cold analysis dominates a first run.** `.pristine.i64` turns ~30s of auto-analysis + into a file copy; if it is missing or older than the binary it is rebuilt. A suite that + suddenly takes minutes longer is usually rebuilding that cache. +- **Under load, idalib gets SIGKILLed mid-analysis** and it looks like a hang or empty + output. Check `uptime` before believing a failure. +- **`experiments/*.py` need `PYTHONPATH=$PWD`** under `~/ida-venv/bin/python` (they are + scripts, not a package entry point). + +## Comparing against master (backend A/B) + +The port replaces the whole backend, so "is this a regression?" means running the same +scenario on both. `git stash -u; git checkout master; <run>; git checkout -; git stash pop` +works, but **use a different scratch binary per branch** (`/tmp/x_master`, `/tmp/x_port`) +so the two runs never share a `.pristine.i64` or a live Code Mode instance. |