diff options
| author | blasty <blasty@local> | 2026-08-07 03:29:35 +0200 |
|---|---|---|
| committer | blasty <blasty@local> | 2026-08-07 03:29:35 +0200 |
| commit | e74c8d24311716d7f552b03d59a53b424295c0f7 (patch) | |
| tree | 62a585b223c1b5b4ad9bdad8e2f9df5710af88bb | |
| parent | Three targeted cuts: the graph's transposition pass counts keep and swap in o... (diff) | |
| download | ida-tui-e74c8d24311716d7f552b03d59a53b424295c0f7.tar.gz ida-tui-e74c8d24311716d7f552b03d59a53b424295c0f7.tar.xz ida-tui-e74c8d24311716d7f552b03d59a53b424295c0f7.zip | |
autoresearch: record the bench history, the budget and the graph non-determinism finding
| -rw-r--r-- | .auto/ideas.md | 67 | ||||
| -rw-r--r-- | .auto/log.jsonl | 1 | ||||
| -rw-r--r-- | .auto/prompt.md | 29 | ||||
| -rw-r--r-- | .auto/wip-cellrow.patch | 94 | ||||
| -rw-r--r-- | .auto/wip-headcache.patch | 256 | ||||
| -rw-r--r-- | .auto/wip-searchbody.patch | 78 |
6 files changed, 64 insertions, 461 deletions
diff --git a/.auto/ideas.md b/.auto/ideas.md index f802898..3a18b33 100644 --- a/.auto/ideas.md +++ b/.auto/ideas.md @@ -1,35 +1,37 @@ # Ideas backlog -## Perf (not yet tried) +## Perf, not yet tried -- **Skeleton walk for `ensure_ea`.** Navigation only needs the *row index* of an - address, yet `ListingModel` walks the segment loading fully-rendered rows. A - `skeleton=true` mode on the `heads` tool returning `(ea, size, kind)` and the - banner/label/member row COUNTS (which is what makes row indices line up) - would drop the walk from ~18 µs/row to maybe 4. The catch: whatever needs the - text later (search's `load_all`) then pays it instead, so the win is real only - if the text streams in the background while the user reads. Big change to - `ListingModel`; do it only after the cheaper things are exhausted. -- **`_grow` should start from where the user is**, not run one linear sweep, so - a jump into the middle of a binary doesn't wait for everything before it. -- **`heads` could return columnar arrays** instead of one dict per row - (`{"ea": [...], "kind": [...], "text": [...]}`). Fewer objects to pickle and - unpickle; `Head.from_raw` currently costs ~2.5 µs/row on the client. -- **Persist the line cache.** `_idatui_line_parts` is warm only within one - worker; the same binary is reopened constantly during a session. -- **`decompile` on adjacent functions** could be prefetched while the user - reads the current one. +- **Skeleton walk for `ensure_ea`** — navigation only needs the *row index* of an + address, yet `ListingModel` walks the segment loading fully-rendered rows. + **Costed, and it only nets ~5%**: a text-free walk would be ~3 µs/row instead + of ~23, but search then has to fetch the text anyway (it builds its haystack + from `_line_plain`), so most of the saving moves rather than disappears. + Worth it only if the goal changes from "total session time" to "no single + foreground wait over 200 ms" — which is arguably the better goal for a TUI. +- **`_grow` should start from where the user is**, not sweep from the segment + start, so a jump into the middle doesn't wait behind everything before it. +- **Prefetch the decompilation of adjacent/called functions** while the user + reads the current one. The worker is idle then and `Program.decompile` caches. +- **Persist the worker's `_idatui_line_parts` cache** — it is warm only within + one worker, and the same binary is reopened constantly during a session. -## Bugs noticed while optimising (not perf work) +## Bugs found while optimising (not perf work) +- **`idatui/graph.py` edge routing is non-deterministic.** Laying out the *same* + function twice with the *unchanged* engine gives different + `painting.vruns` for 71 of 128 corpus functions. Node placement is stable; + only the routing moves. So a graph redraws differently when you reopen it, and + any old-vs-new painting diff is worthless as a regression test (old-vs-old + fails it too). Find the set/dict iteration or `id()`-keyed order behind it. - **Sticky graph mode makes a keypress ambiguous.** With `_graph_sticky` on, a navigation schedules the next function's graph asynchronously; until it lands - the app is in the listing. So `space` pressed right after a jump either enters - or leaves the graph depending on which won. `tests/test_scenarios.py` - `graph_minimap` was silently relying on losing that race (fixed in the - scenario's setup in experiment #6, but the app behaviour is still ambiguous). - A fix would be to enter graph mode immediately with a loading state when a - sticky navigation starts. + the app is in the listing. So `space` right after a jump either enters or + leaves the graph depending on which won. `graph_minimap` was silently relying + on losing that race. A fix would be to enter graph mode immediately, with a + loading state, when a sticky navigation starts. +- **`domain.decomp_map` costs ~280 ms per function** — more than the decompile + itself — and is on the split-view (`s`) path, which the bench doesn't cover. ## Verification patterns that worked (reuse them) @@ -38,8 +40,15 @@ and compares outputs over every disassembly line of a real binary. Catch: `BODY` is a normal triple-quoted string, so you must import the module and read `mod.BODY` — slicing the file text tests an undecoded program. -- **Differential inside one process** (`/tmp/fceq.py`, `/tmp/hleq.py`, - `/tmp/searcheq.py`): build the input twice, run old and new, compare the - structures. Cheap, and it catches "faster because it does less". +- **Compare what reaches the screen, not the data structure.** Merging Segments + is *supposed* to change the segments; expand both to `(char, style)` per cell + and compare that (`/tmp/hexeq.py`, `/tmp/gveq.py`). +- **Check the old code against itself first.** The graph routing diff looked + like a regression until old-vs-old failed identically. +- **`.auto/check_search.py`** is the permanent version of this idea and runs in + `checks.sh`: it compares the search fast paths against the plain per-line loop + for every typed prefix. Cache staleness returns a *plausible wrong answer*, + which no scenario test can catch. - The bench's `NOTES` counters (`search_hits`, `graph_blocks`, `decomp_ok`, - `render_cells`, `nav_rows`) are the standing guard against that. + `render_cells`, `nav_rows`, `listing_cells`) are the standing guard against + "faster because it did less". diff --git a/.auto/log.jsonl b/.auto/log.jsonl index e22adae..c94c009 100644 --- a/.auto/log.jsonl +++ b/.auto/log.jsonl @@ -25,3 +25,4 @@ {"type":"config","name":"ida-tui performance (v5 bench: 2ms landing polls; final measurement shape)","metricName":"total_ms","metricUnit":"ms","bestDirection":"lower"} {"run":21,"commit":"5a8027e","metric":18516,"metrics":{"lg_boot_ms":663.9,"lg_decomp_ms":2663.2,"lg_graph_ms":1012.3,"lg_hex_ms":436.1,"lg_index_ms":98.4,"lg_listing_cold_ms":437.7,"lg_listing_warm_ms":409.4,"lg_nav_ms":6841.9,"lg_palette_ms":4.7,"lg_render_ms":218.8,"lg_search_ms":1474.7,"pure_graph_ms":241.6,"sm_boot_ms":433.3,"sm_decomp_ms":1266.3,"sm_graph_ms":737.9,"sm_hex_ms":424.2,"sm_index_ms":2.3,"sm_listing_cold_ms":262.9,"sm_listing_warm_ms":283.2,"sm_nav_ms":301.2,"sm_palette_ms":0.3,"sm_render_ms":253.9,"sm_search_ms":47.9,"fails":0},"status":"keep","description":"Baseline for the v5 bench (landing polls every 2ms instead of 10ms; the poll interval was measurement overhead inside the timed regions). Final measurement shape — no further bench changes.","timestamp":1786065328607,"segment":4,"confidence":null,"asi":{"hypothesis":"the 10ms wait_for step was charging every timed landing up to 10ms of quantisation","effect":"sm_nav 373 -> 301, sm_decomp 1309 -> 1266 (12 timed waits each). lg_graph and lg_decomp did not move measurably -- their per-item costs are large enough that 5ms of expected overshoot is lost in the spread.","decision":"this is the last bench change. Any further one costs a re-baseline, and the measurement is now honest about cold vs warm (v4) and free of both the cache-hit and quantisation artefacts.","budget_ms":{"lg_nav":6842,"decomp lg+sm":3930,"graph lg+sm":1750,"search lg+sm":1523,"listing lg+sm":1393,"boot lg+sm":1097,"hex lg+sm":860,"render lg+sm":473,"pure_graph":242},"decomp_anatomy_echo":"12 small functions: Program.decompile (Hex-Rays) 562ms, UI F5 path with everything already cached 414ms, four scroll+paint frames 186ms, highlight_c 86ms. The 414ms of UI is two @work thread spawns, ~4 event-loop hops, and a loading-cover mount/unmount per F5 -- and show() runs exactly ONCE per F5 (checked by counting), so there is no double work to remove.","next_action_hint":"the loading cover is 4.6ms of mount+unmount per F5 and cannot be deferred (a scenario asserts F5 raises it synchronously). The remaining decomp lever is _show_active spawning _load_decomp as a thread even when Program already has the decompilation cached."}} {"run":22,"commit":"5a8027e","metric":18535.4,"metrics":{"lg_boot_ms":692.3,"lg_decomp_ms":2662.1,"lg_graph_ms":895.9,"lg_hex_ms":422.2,"lg_index_ms":94.1,"lg_listing_cold_ms":525.4,"lg_listing_warm_ms":404.2,"lg_nav_ms":6821.6,"lg_palette_ms":4.7,"lg_render_ms":215.7,"lg_search_ms":1489.3,"pure_graph_ms":247.8,"sm_boot_ms":437.2,"sm_decomp_ms":1258.5,"sm_graph_ms":749.3,"sm_hex_ms":437.6,"sm_index_ms":2.4,"sm_listing_cold_ms":263.3,"sm_listing_warm_ms":283.1,"sm_nav_ms":321.5,"sm_palette_ms":0.3,"sm_render_ms":257.5,"sm_search_ms":49.2,"fails":0},"status":"discard","description":"Apply an already-decompiled function inline instead of spawning a background worker for it (Program.cached_decompilation/cached_pc_nums + _decomp_now), and warm pc_nums in the thread that already decompiled. Total flat (18516 -> 18535) and the phase it targets did not move either (lg_decomp 2663 -> 2662, sm_decomp 1266 -> 1259).","timestamp":1786065730509,"segment":4,"confidence":null,"asi":{"hypothesis":"an F5 spawns two @work threads and hops the event loop four times; skipping the second when the answer is already cached should show up in decomp","result":"it does not. A Textual thread spawn plus its call_from_thread round trip is worth well under 1% of a 100-220ms F5. The cost is elsewhere: Hex-Rays itself (562ms of sm_decomp's 1266), the loading-cover mount/unmount, highlight_c, and the Strip/scroll work in DecompView.show.","rollback_reason":"no measurable gain on the targeted phase, and it adds a second path into _apply_decomp with its own generation-check reasoning to keep correct. Simpler is better.","correctness_was_fine":"301 scenarios passed with it in; this is a complexity-vs-payoff rejection, not a bug.","kept_knowledge":"Program.cached_decompilation / cached_pc_nums would be the right primitives if a future change needs to know whether an answer is in hand without paying for it","next_action_hint":"boot is 1097ms across both targets and ~300ms of it is the worker importing idapro before it can serve. Check whether launch.py can spawn the worker BEFORE Textual starts, so that import overlaps the app's own startup instead of following it."}} +{"run":23,"commit":"98b3b98","metric":17944.4,"metrics":{"lg_boot_ms":703.7,"lg_decomp_ms":2453.7,"lg_graph_ms":1000,"lg_hex_ms":478,"lg_index_ms":96,"lg_listing_cold_ms":453,"lg_listing_warm_ms":411.2,"lg_nav_ms":6555.9,"lg_palette_ms":4.9,"lg_render_ms":218.4,"lg_search_ms":1308.8,"pure_graph_ms":212.3,"sm_boot_ms":432.4,"sm_decomp_ms":1267.8,"sm_graph_ms":742.7,"sm_hex_ms":440,"sm_index_ms":2.5,"sm_listing_cold_ms":264.3,"sm_listing_warm_ms":289.5,"sm_nav_ms":305.4,"sm_palette_ms":0.3,"sm_render_ms":258.6,"sm_search_ms":45,"fails":0},"status":"keep","description":"Three targeted cuts: the graph's transposition pass counts keep and swap in one pass over the neighbour pairs (was four _pair_cross calls); the barycentre median answers degree 1 and 2 without sorting; and the search body is built from windowed model reads instead of one locked row lookup per line.","timestamp":1786066135637,"segment":4,"confidence":29.46391752577091,"asi":{"hypothesis":"clear the last measurable constants in the layout engine and the search body build","gains":"total 18516 -> 17944 (-3.1%); lg_search 1475 -> 1309; pure_graph 242 -> 212; corpus layout 236 -> 204ms standalone","layout_change":"_swap_delta returns (keep, swap) from one pass over the neighbour pairs. _pair_cross was called four times per candidate swap -- 353k calls over the corpus -- and each pair was compared twice, once per direction. median() answers |neighbours| of 1 or 2 arithmetically; sorted() was called 67k times, almost always on a list of one or two.","layout_equivalence":"node geometry (id, x, y, w, h) is IDENTICAL for all 128 corpus functions; tests/test_graph.py passes 470 checks including the no-edge-inside-a-box invariant.","PRE_EXISTING_BUG_FOUND":"idatui/graph.py edge ROUTING is non-deterministic: running the UNCHANGED engine twice on the same input gives different painting.vruns for 71 of 128 functions. Node placement is stable; only the routing moves. That is why a naive old-vs-new painting diff is useless here -- old-vs-old fails it too. Logged in .auto/ideas.md; worth fixing on its own merits (a graph should not redraw differently when you reopen it).","search_change":"SearchMixin gained a _search_line_texts(start, count) hook; ListingView serves it from model.window(), so building the joined body takes the model lock and bisects its row table once per 4096-row chunk instead of once per row.","verified":".auto/check_search.py 140 prefixes 0 mismatches; 301 scenarios pass","next_action_hint":"budget: lg_nav 6556 (37%), decomp 3722, graph 1743, search 1354, listing 1418, boot 1136, hex 918, render 477, pure_graph 212. Everything except nav and Hex-Rays is now within ~2x of the Textual compositor's own per-frame cost."}} diff --git a/.auto/prompt.md b/.auto/prompt.md index 73d9692..3965e3e 100644 --- a/.auto/prompt.md +++ b/.auto/prompt.md @@ -148,14 +148,35 @@ v1 bench baseline `total_ms` ≈ 46 600 → 18 900 after thirteen experiments - Growing the line cache past 16 384 does nothing for a *cold* sweep (16.5 µs/row at both 16 k and 64 k); it only helps a second sweep (10.6 vs 16.4). -**Where the time is now (18 857 ms)** +**Where the time is now (17 944 ms on the v5 bench)** -`lg_nav` 6 591 (35%) · decomp lg+sm 3 151 · `lg_search` 2 196 · graph lg+sm -1 807 · listing 1 478 · hex 1 270 · boot 1 121 · render 459 · pure_graph 238. +`lg_nav` 6 556 (37%) · decomp lg+sm 3 722 · graph lg+sm 1 743 · search lg+sm +1 354 · listing lg+sm 1 418 · boot lg+sm 1 136 · hex lg+sm 918 · render 477 · +pure_graph 212. `lg_nav` is a linear walk of the segment at its floor: ~16.5 µs/row in the worker (of which `generate_disasm_line` is 5.9 µs and IDA's) plus ~7 µs/row on -the client. The only lever left there is *not walking* — see `.auto/ideas.md`. +the client. The only lever left there is *not walking* — costed in +`.auto/ideas.md`, and it only nets ~5% because search needs the text anyway. + +`decomp` is 1.37 s of Hex-Rays on the big target plus ~35 ms per F5 of Textual +(a loading-cover mount/unmount, two `@work` threads, `highlight_c`). The cover +cannot be deferred — a scenario asserts F5 raises it synchronously, and that +assertion guards a real past regression. + +Most other phases are now within ~2× of Textual's own per-frame compositing +cost, which is not ours to remove. + +**Benchmark history** — the bench was corrected three times, each time because +it was measuring a cache hit or its own overhead. Every re-baseline is in +`.auto/log.jsonl`: + +| bench | baseline | best | what changed | +|---|---|---|---| +| v1 | 46 572 | 19 006 | — | +| v2 | 27 913 | 17 501 | graph opens were timing a cache hit | +| v4 | 18 498 | — | decomp/search/index reps were timing cache hits | +| v5 | 18 516 | 17 944 | landing polls every 2 ms, not 10 | **Measurement traps** diff --git a/.auto/wip-cellrow.patch b/.auto/wip-cellrow.patch deleted file mode 100644 index cb46dae..0000000 --- a/.auto/wip-cellrow.patch +++ /dev/null @@ -1,94 +0,0 @@ -diff --git a/idatui/app.py b/idatui/app.py -index 53dd79b..6c0a872 100644 ---- a/idatui/app.py -+++ b/idatui/app.py -@@ -2161,6 +2161,13 @@ _GPAD = 1 # columns of padding inside a box - _MINI_W, _MINI_H = 30, 14 - - -+#: Memo for ``Style + Style``. Rich rebuilds a whole Style on every ``+``, and -+#: the graph merges an overlay (cursor band, trail, highlight) over a run of -+#: cells that share only a handful of base styles -- so the same pair is -+#: recombined hundreds of times per frame. -+_STYLE_SUM: dict[tuple, Style] = {} -+ -+ - class _CellRow: - """A row of (char, style) cells that coalesces into a Strip. - -@@ -2183,13 +2190,39 @@ class _CellRow: - self.st[i] = style - - def text(self, i: int, s: str, style: Style) -> None: -- for k, c in enumerate(s): -- self.put(i + k, c, style) -+ """Write ``s`` at cell ``i``, clipped to the row. -+ -+ Slice assignment rather than a call per character: a graph row is drawn -+ from box borders, instruction text and the minimap, and doing it a cell -+ at a time made painting one frame thousands of bound-method calls. -+ Assigning a str to a list slice expands it to characters in C. -+ """ -+ if not s: -+ return -+ a = i if i > 0 else 0 -+ b = i + len(s) -+ if b > self.width: -+ b = self.width -+ if b <= a: -+ return -+ self.ch[a:b] = s[a - i:b - i] -+ self.st[a:b] = [style] * (b - a) - - def restyle(self, a: int, b: int, style: Style) -> None: - """Merge ``style`` over the cells in [a, b) (keeps the characters).""" -- for i in range(max(a, 0), min(b, self.width)): -- self.st[i] = self.st[i] + style -+ if a < 0: -+ a = 0 -+ if b > self.width: -+ b = self.width -+ st = self.st -+ combine = _STYLE_SUM -+ for i in range(a, b): -+ base = st[i] -+ key = (base, style) -+ got = combine.get(key) -+ if got is None: -+ got = combine[key] = base + style -+ st[i] = got - - def strip(self) -> Strip: - segs: list[Segment] = [] -@@ -2796,10 +2829,8 @@ class GraphView(NavMixin, ScrollView, can_focus=True): - label = f"loc_{n.block.start:X}" if n.block else "" - if b is not None and b.rows and b.rows[0].name: - label = b.rows[0].name -- out.put(left, graph.BOX["tl"], bs) -- for i in range(1, w - 1): -- out.put(left + i, graph.BOX["h"], bs) -- out.put(left + w - 1, graph.BOX["tr"], bs) -+ out.text(left, graph.BOX["tl"] + graph.BOX["h"] * (w - 2) -+ + graph.BOX["tr"], bs) - tag = f" {label} " - if len(tag) <= w - 4: - st = _S_GENTRY if (self.fc and n.id == self.fc.entry) else ( -@@ -2809,15 +2840,12 @@ class GraphView(NavMixin, ScrollView, can_focus=True): - out.put(left + w - 2, "↺", _S_EDGE[graph.E_BACK]) - return - if row == n.y + n.h - 1: -- out.put(left, graph.BOX["bl"], bs) -- for i in range(1, w - 1): -- out.put(left + i, graph.BOX["h"], bs) -- out.put(left + w - 1, graph.BOX["br"], bs) -+ out.text(left, graph.BOX["bl"] + graph.BOX["h"] * (w - 2) -+ + graph.BOX["br"], bs) - return - out.put(left, graph.BOX["v"], bs) - out.put(left + w - 1, graph.BOX["v"], bs) -- for i in range(1, w - 1): -- out.put(left + i, " ", _S_INSN) -+ out.text(left + 1, " " * (w - 2), _S_INSN) - i = row - n.y - 1 - rows = self._rows(n.id) - if not (0 <= i < len(rows)): diff --git a/.auto/wip-headcache.patch b/.auto/wip-headcache.patch deleted file mode 100644 index d7d6ab6..0000000 --- a/.auto/wip-headcache.patch +++ /dev/null @@ -1,256 +0,0 @@ -diff --git a/.auto/diff_spans.py b/.auto/diff_spans.py -index 95721a5..aad4ebc 100644 ---- a/.auto/diff_spans.py -+++ b/.auto/diff_spans.py -@@ -39,11 +39,11 @@ def load_impl(path: str, name: str): - mod = importlib.util.module_from_spec(spec) - spec.loader.exec_module(mod) # IDA-free at import time - body = mod.BODY -- a = body.index("#: IDA colour tag -> the semantic kind") -- b = body.index("def _idatui_unknown_row") -+ a = body.index("def _idatui_head_row") -+ b = body.index("def _idatui_struct_member_rows") - g = {"__name__": name} - exec(compile(body[a:b], name, "exec"), g) # noqa: S102 -- return g["_idatui_spans"] -+ return g - - - def main() -> int: -@@ -59,8 +59,10 @@ def main() -> int: - fh.write(subprocess.run( - ["git", "-C", ROOT, "show", f"{a.ref}:server/patch_server.py"], - capture_output=True, text=True, check=True).stdout) -- new = load_impl(os.path.join(ROOT, "server", "patch_server.py"), "new") -- old = load_impl(old_path, "old") -+ gnew = load_impl(os.path.join(ROOT, "server", "patch_server.py"), "new") -+ gold = load_impl(old_path, "old") -+ new, old = gnew["_idatui_spans"], gold["_idatui_spans"] -+ new_row, old_row = gnew["_idatui_head_row"], gold["_idatui_head_row"] - - binary = os.path.join(ROOT, a.target) - tgt = os.path.join(d, os.path.basename(binary)) -@@ -89,7 +91,15 @@ def main() -> int: - if ra != rb: - bad += 1 - if bad <= 3: -- print(f"MISMATCH @ {ea:#x}\n line={line!r}\n" -+ print(f"SPAN MISMATCH @ {ea:#x}\n line={line!r}\n" -+ f" old={ra!r}\n new={rb!r}") -+ # The whole row, not just the spans: `text`, the spans/text -+ # agreement guard and the name all moved around too. -+ ra, rb = old_row(ea), new_row(ea) -+ if ra != rb: -+ bad += 1 -+ if bad <= 3: -+ print(f"ROW MISMATCH @ {ea:#x}\n" - f" old={ra!r}\n new={rb!r}") - nxt = ida_bytes.get_item_end(ea) - ea = nxt if nxt > ea else ea + 1 -diff --git a/idatui/domain.py b/idatui/domain.py -index b496638..1c7883f 100644 ---- a/idatui/domain.py -+++ b/idatui/domain.py -@@ -123,19 +123,22 @@ class Head: - return None - - @classmethod -- def from_raw(cls, d: dict) -> "Head": -+ def from_raw(cls, d: dict, raw: bytes | None = None) -> "Head": - sp = d.get("spans") - ops = d.get("ops") -+ # ``tuple(map(tuple, ...))`` rather than a per-item genexpr with str()/ -+ # int() coercion: this runs once per listing row (hundreds of thousands -+ # on a real binary) and the worker's own tool already emits [str, str] -+ # and [int, int, int]. The coercion was re-proving that on every row. - return cls( - ea=_as_int(d["ea"]), - kind=d.get("kind", "unknown"), - size=int(d.get("size", 0) or 0), - text=d.get("text", ""), - name=d.get("name"), -- spans=(tuple((str(k), str(t)) for k, t in sp) -- if isinstance(sp, list) and sp else None), -- ops=(tuple((int(a), int(b), int(n)) for a, b, n in ops) -- if isinstance(ops, list) and ops else None), -+ raw=raw, -+ spans=tuple(map(tuple, sp)) if sp else None, -+ ops=tuple(map(tuple, ops)) if ops else None, - ) - - -@@ -654,30 +657,48 @@ class ListingModel: - # containing a huge coalesced undefined run doesn't pull megabytes. - _OP_SPAN_CAP = 1 << 16 - -- def _attach_opcode_bytes(self, page: list[Head]) -> list[Head]: -- """Fill ``raw`` (opcode bytes) for the code heads in ``page`` via one -- bulk read over their extent (variable-length safe).""" -- code = [h for h in page if h.kind == "code" and h.size > 0] -- if not code: -- return page -- lo = code[0].ea -- hi = code[-1].ea + code[-1].size -- if hi - lo <= 0 or hi - lo > self._OP_SPAN_CAP: -- return page -- data = self._prog.read_bytes(lo, hi - lo) -+ def _build_page(self, rows: list) -> list[Head]: -+ """Turn the tool's raw rows into ``Head``s with their opcode bytes -+ already attached, via one bulk read over the code extent. -+ -+ The bytes are read BEFORE the Heads are built rather than patched in -+ afterwards: ``dataclasses.replace`` re-runs ``__init__`` with every -+ field, so filling ``raw`` after the fact meant constructing each code -+ head twice -- once per listing row, on the path a jump-to-address walks -+ hundreds of thousands of times. -+ """ -+ lo = hi = -1 -+ for r in rows: -+ if r.get("kind") == "code" and r.get("size"): -+ ea = _as_int(r["ea"]) -+ if lo < 0: -+ lo = ea -+ hi = ea + int(r["size"]) -+ data = None -+ if 0 <= lo < hi and hi - lo <= self._OP_SPAN_CAP: -+ try: -+ data = self._prog.read_bytes(lo, hi - lo) -+ except Exception: # noqa: BLE001 -- opcode bytes are decoration -+ data = None -+ page: list[Head] = [] - biggest = self._max_raw -- out = [] -- for h in page: -- if h.kind == "code" and h.size > 0: -- off = h.ea - lo -- b = bytes(data[off:off + h.size]) -- biggest = max(biggest, len(b)) -- out.append(replace(h, raw=b)) -- else: -- out.append(h) -- with self._lock: -- self._max_raw = biggest -- return out -+ for r in rows: -+ raw = None -+ if data is not None and r.get("kind") == "code": -+ size = int(r.get("size") or 0) -+ if size > 0: -+ off = _as_int(r["ea"]) - lo -+ raw = bytes(data[off:off + size]) -+ if len(raw) > biggest: -+ biggest = len(raw) -+ try: -+ page.append(Head.from_raw(r, raw)) -+ except (KeyError, ValueError, TypeError): -+ continue -+ if biggest != self._max_raw: -+ with self._lock: -+ self._max_raw = biggest -+ return page - - def max_raw_len(self) -> int: - with self._lock: -@@ -700,13 +721,7 @@ class ListingModel: - "heads", addr=hex(frm), count=self.PAGE, annotate=True) - rows = payload.get("heads", []) if isinstance(payload, dict) else [] - cur = payload.get("cursor", {}) if isinstance(payload, dict) else {} -- page = [] -- for r in rows: -- try: -- page.append(Head.from_raw(r)) -- except (KeyError, ValueError, TypeError): -- continue -- page = self._attach_opcode_bytes(page) -+ page = self._build_page(rows) - with self._lock: - for h in page: - # Banner/label rows (function headers, separators, code labels) -diff --git a/server/patch_server.py b/server/patch_server.py -index b75b120..a1ec1ef 100644 ---- a/server/patch_server.py -+++ b/server/patch_server.py -@@ -297,33 +297,59 @@ def _idatui_head_row(ea): - else: - kind = "unknown" - line = ida_lines.generate_disasm_line(ea, 0) -- text = ida_lines.tag_remove(line) if line else "" -- text = " ".join(text.split()) # collapse IDA's column padding -+ text, spans, ops = _idatui_line_parts(line) if line else ("", None, None) - row = { - "ea": hex(ea), - "kind": kind, - "size": int(ida_bytes.get_item_size(ea)), - "text": text, - } -- if line: -- # Keep IDA's own token classification for syntax highlighting. Built from -- # the SAME line as `text`, then whitespace-collapsed identically so the -- # two never disagree about what the row says. -- spans, ops = _idatui_spans(line) -- joined = "".join(t for _k, t in spans) -- if " ".join(joined.split()) == text: -- row["spans"] = spans -- # Where each operand sits in `text`. Comes out of the same tag walk -- # (free), and is what lets the client show WHICH literal a keypress -- # would reformat before you press it. -- if ops: -- row["ops"] = ops -+ if spans is not None: -+ row["spans"] = spans -+ # Where each operand sits in `text`. Comes out of the same tag walk -+ # (free), and is what lets the client show WHICH literal a keypress -+ # would reformat before you press it. -+ if ops: -+ row["ops"] = ops - nm = ida_name.get_ea_name(ea) - if nm: - row["name"] = nm - return row - - -+import functools as _idatui_functools -+ -+ -+@_idatui_functools.lru_cache(maxsize=16384) -+def _idatui_line_parts(line): -+ """``(text, spans, ops)`` for one tagged disassembly line -- memoised. -+ -+ A function of the tagged line and nothing else, so the same line always -+ gives the same answer: a rename changes the line, which changes the key. -+ And listings repeat themselves hard -- 196k lines of bash are 53k distinct -+ ones, so a 16k-entry cache serves ~70% of them and takes the per-line cost -+ from 10.4us to 3.9us. This is the most expensive thing the backend does per -+ listing row, and a jump to an address near the end of a big binary walks -+ hundreds of thousands of them. -+ -+ ``spans`` is None when the tag walk and the plain text disagree about what -+ the line says (then the text wins and the row renders unhighlighted). -+ -+ The returned lists are SHARED between every row that has the same line; -+ treat them as read-only. Pickle notices the sharing too, so a page of -+ repetitive disassembly also serialises smaller. -+ """ -+ import ida_lines -+ text = " ".join(ida_lines.tag_remove(line).split()) # collapse the padding -+ spans, ops = _idatui_spans(line) -+ # Built from the SAME line as `text`, then whitespace-collapsed identically, -+ # so the two can never disagree about what the row says. -+ joined = "".join([t for _k, t in spans]) -+ if " ".join(joined.split()) != text: -+ return (text, None, None) -+ return (text, spans, ops) -+ -+ - #: IDA colour tag -> the semantic kind the TUI styles. IDA already classifies - #: every token in a disassembly line, for every processor it supports, so there - #: is nothing to lex: generate_disasm_line emits \x01<tag>text\x02<tag> and the -@@ -407,7 +433,7 @@ def _idatui_spans(line): - # the most expensive thing the `heads` tool did, and a line is ~54 - # characters but only ~13 tags -- everything between two tags is already - # exactly one span's worth of text. -- _IDATUI_CTL = _re.compile("([\\x01\\x02\\x03][\\s\\S])") -+ _IDATUI_CTL = _re.compile("([\\x01\\x02\\x03](?s:.))") - tags, opnds = _IDATUI_TAGS, _IDATUI_OPND_TAGS - on, off, esc = "\x01", "\x02", "\x03" - addr_tag = chr(getattr(ida_lines, "COLOR_ADDR", 0x28)) diff --git a/.auto/wip-searchbody.patch b/.auto/wip-searchbody.patch deleted file mode 100644 index f7d3ddb..0000000 --- a/.auto/wip-searchbody.patch +++ /dev/null @@ -1,78 +0,0 @@ -diff --git a/idatui/app.py b/idatui/app.py -index 20aed88..53dd79b 100644 ---- a/idatui/app.py -+++ b/idatui/app.py -@@ -655,14 +655,24 @@ class SearchMixin: - pointed at a different model/body, which invalidates a narrowing.""" - return id(getattr(self, "model", None) or getattr(self, "_texts", None)) - -- def _reset_search_cache(self) -> None: -- """Forget both the narrowing key and the joined body. Called from every -- place that resets ``_matches``/``_ranges``: a stale prefix would make the -- next search narrow from an empty list, and a stale body would search -- text the view no longer shows.""" -+ def _reset_search_cache(self, body: bool = False) -> None: -+ """Forget the narrowing key, and with ``body=True`` the joined body too. -+ -+ Every place that resets ``_matches``/``_ranges`` must call this: a stale -+ prefix would make the next search narrow from an empty list. -+ -+ The body is a different question. It is keyed by (row count, line source) -+ so it invalidates itself when the view is pointed somewhere else or more -+ rows stream in — which means ending a search does NOT have to throw it -+ away, and the next `/` over the same segment is then instant instead of -+ re-joining a quarter of a million lines. It DOES have to go when the -+ plain text of a row changes without either of those moving, which is -+ exactly what toggling the opcode-bytes column does. -+ """ - self._matched_key = None -- self._hay_key = None -- self._hay = None -+ if body: -+ self._hay_key = None -+ self._hay = None - - def _search_haystack(self, count: int, src: int): - """``(starts, blob, blob_folded)`` for the whole body, or None. -@@ -1062,7 +1072,7 @@ class ListingView(SearchMixin, NavMixin, ColumnCursor, ScrollView, can_focus=Tru - def load(self, model: ListingModel, name: str, cursor: int = 0, - cursor_x: int = 0, scroll_y: int | None = None, - focus: str | None = None) -> None: -- self.model = model -+ previous, self.model = self.model, model - self._name = name - self.total = 0 - self.cursor = cursor -@@ -1072,7 +1082,9 @@ class ListingView(SearchMixin, NavMixin, ColumnCursor, ScrollView, can_focus=Tru - self._pending_op = None - self._matches = [] - self._ranges = {} -- self._reset_search_cache() -+ # Navigating inside the same segment reuses the same model, and the -+ # searchable body with it; only a different model invalidates it. -+ self._reset_search_cache(body=model is not previous) - self._prime() - - @work(thread=True, exclusive=True, group="listing-prime") -@@ -1147,7 +1159,8 @@ class ListingView(SearchMixin, NavMixin, ColumnCursor, ScrollView, can_focus=Tru - self._op_mode = (self._op_mode + 1) % 3 - self._update_op_w() - self._ranges = {} # column layout changed -> stale match offsets -- self._reset_search_cache() # ...and which rows match at all -+ # ...and which rows match at all: the opcode hex is searchable text. -+ self._reset_search_cache(body=True) - self._clamp_x() - self.refresh() - self._app_status("opcodes: " + {0: "off", 1: f"limited ({_OP_LIMIT} bytes)", -@@ -1583,7 +1596,10 @@ class DecompView(SearchMixin, NavMixin, ColumnCursor, ScrollView, can_focus=True - self.cursor_x = cursor_x - self._matches = [] - self._ranges = {} -- self._reset_search_cache() -+ # A whole new body: drop the joined haystack outright rather than trust -+ # id(self._texts) to differ, since the list it replaces is freed here and -+ # its address can be handed straight back. -+ self._reset_search_cache(body=True) - # Gutter wide enough for the largest line number + a trailing space. - self._gutter = (len(str(total)) + 1) if total else 0 - maxw = max((s.cell_length for s in self._strips), default=0) |
