diff options
| -rw-r--r-- | .auto/log.jsonl | 1 | ||||
| -rw-r--r-- | idatui/domain.py | 138 |
2 files changed, 134 insertions, 5 deletions
diff --git a/.auto/log.jsonl b/.auto/log.jsonl index 68e1473..951b62f 100644 --- a/.auto/log.jsonl +++ b/.auto/log.jsonl @@ -35,3 +35,4 @@ {"run":29,"commit":"7ea3ca1","metric":33502.3,"metrics":{"lg_boot_ms":772.2,"lg_decomp_ms":2546.9,"lg_graph_ms":903.4,"lg_hex_ms":434.1,"lg_index_ms":97.7,"lg_listing_cold_ms":420.2,"lg_listing_warm_ms":397.4,"lg_nav_ms":6501.1,"lg_palette_ms":4.9,"lg_render_ms":212.4,"lg_search_ms":760.4,"lg_split_ms":10189.2,"pure_graph_ms":212.3,"sm_boot_ms":443.8,"sm_decomp_ms":1275.3,"sm_graph_ms":720.5,"sm_hex_ms":451,"sm_index_ms":2.5,"sm_listing_cold_ms":263.3,"sm_listing_warm_ms":264.7,"sm_nav_ms":302.8,"sm_palette_ms":0.3,"sm_render_ms":248.4,"sm_search_ms":43.9,"sm_split_ms":6033.6,"fails":0},"status":"keep","description":"RE-BASELINE (v6 bench). The split view ('s') was not covered at all, and it turns out to be the most expensive thing in the app: 16.2s of a 33.5s session (lg_split 10189 + sm_split 6034), or 500ms per function on echo and 850ms on bash, just to open it.","timestamp":1786070710925,"segment":5,"confidence":null,"asi":{"hypothesis":"the bench covers listing, decomp, graph, hex, search, nav and boot -- but not the split view, and an earlier aside measured decomp_map at 280ms per function","finding":"split is 48% of the whole benchmarked session once measured. 12 echo functions take 6.0s to open side by side; 12 bash functions take 10.2s.","why_it_was_missed":"the phase list was written from the README's headline features and 's' was not one of them. Same class of gap as v2 (graph opens timed a cache hit): if a feature is not in the bench, its cost is invisible however carefully you profile the ones that are.","cause_already_diagnosed":"server/patch_server.py decomp_map sweeps EVERY COLUMN of every pseudocode line, allocating three ctree_item_t SWIG objects per column and calling item.dstr() (which formats a whole 'EA: description' string) each time. Three separate wastes: the allocations, sweeping len(sl.line) which is the TAGGED length (124 columns for a 23-column line), and re-formatting for columns that report the same ctree item.","fix_ready":".auto/wip-decompmap.patch -- verified byte-identical over 165 functions across echo/ls_ttl/bash, 8.9-9.8x faster with Hex-Rays warm for both sides (bash's 25 largest: 67.4s -> 6.8s of sweeping)","next_action_hint":"apply it"}} {"run":30,"commit":"da1dfe8","metric":19834.8,"metrics":{"lg_boot_ms":676.9,"lg_decomp_ms":2698.8,"lg_graph_ms":892.1,"lg_hex_ms":440.4,"lg_index_ms":93.6,"lg_listing_cold_ms":438.5,"lg_listing_warm_ms":420.8,"lg_nav_ms":6646.7,"lg_palette_ms":4.7,"lg_render_ms":215.1,"lg_search_ms":767,"lg_split_ms":1327.6,"pure_graph_ms":213.6,"sm_boot_ms":468,"sm_decomp_ms":1317.1,"sm_graph_ms":730.7,"sm_hex_ms":483.2,"sm_index_ms":2.6,"sm_listing_cold_ms":266.1,"sm_listing_warm_ms":268.1,"sm_nav_ms":288.6,"sm_palette_ms":0.3,"sm_render_ms":253,"sm_search_ms":47.1,"sm_split_ms":874.2,"fails":0},"status":"keep","description":"decomp_map: stop sweeping every column three times over. It allocated three ctree_item_t SWIG objects PER COLUMN, swept the tagged line length (124 columns for a 23-column line), and called dstr() — which formats a whole 'EA: description' string — for every column even though consecutive columns report the same ctree item. Now: one item, no head/tail, visible columns only, and dstr() only when the item's obj_id changes.","timestamp":1786070905591,"segment":5,"confidence":null,"asi":{"gains":"total 33502 -> 19835 (-40.8%); lg_split 10189 -> 1328 (-87%); sm_split 6034 -> 874 (-86%)","work_unchanged_proof":"split_mapped_lines is 985 (echo) and 2069 (bash) BEFORE and AFTER -- the same per-line instruction sets are produced, from the same number of lines","equivalence":"/tmp/dmapeq2.py calls the shipped tool and a copy of the pre-change implementation for the same functions and compares the whole payload: 0 real mismatches over 165 functions on echo/ls_ttl/bash. (Two apparent ones were my reference's placeholder error string for functions Hex-Rays refuses.) With Hex-Rays warm for both sides the sweep is 8.9-9.8x faster; bash's 25 largest went 67.4s -> 6.8s.","the_three_wastes":["three ctree_item_t SWIG allocations per COLUMN -- one per call is enough, and head/tail are filled but never read, so pass None","range(len(sl.line)) is the TAGGED length: sl.line still carries IDA's colour tags, so a 23-column line was swept 124 times. tag_remove's length is the real bound.","item.dstr() formats a description string for every column; consecutive columns are nearly always the same ctree item. Comparing item.it.obj_id first collapses that to one format per item -- and the result is deduped by `seen` anyway, so skipping a repeat cannot change it."],"lesson_repeated":"this is the third time a cost was invisible because the benchmark did not exercise the feature (graph opens in v2, the non-scenario suites in v5, split view here). Coverage of the FEATURE matters more than precision on the ones already covered.","next_action_hint":"budget now: lg_nav 6647 (34%), decomp 4016, split 2202, graph 1623, listing 1394, boot 1145, hex 924, search 814, render 468. Split is still 2.2s for 24 functions -- what is left there is ida_hexrays.decompile inside decomp_map, which duplicates the decompile the view already did."}} {"type":"config","name":"ida-tui performance (v7 bench: rename covered too)","metricName":"total_ms","metricUnit":"ms","bestDirection":"lower"} +{"run":31,"commit":"7b8c37a","metric":33242.6,"metrics":{"lg_boot_ms":710.8,"lg_decomp_ms":2401.2,"lg_graph_ms":898.1,"lg_hex_ms":448.7,"lg_index_ms":72.1,"lg_listing_cold_ms":439.3,"lg_listing_warm_ms":426.8,"lg_nav_ms":7060.2,"lg_palette_ms":4.8,"lg_rename_ms":10055,"lg_render_ms":220.2,"lg_search_ms":774.6,"lg_split_ms":3539.8,"pure_graph_ms":215.1,"sm_boot_ms":429.1,"sm_decomp_ms":1244.1,"sm_graph_ms":744.3,"sm_hex_ms":444.2,"sm_index_ms":2.5,"sm_listing_cold_ms":269.8,"sm_listing_warm_ms":263.3,"sm_nav_ms":282.9,"sm_palette_ms":0.3,"sm_rename_ms":638.3,"sm_render_ms":255.6,"sm_search_ms":44.2,"sm_split_ms":1357.4,"fails":0},"status":"keep","description":"RE-BASELINE (v7 bench). Rename — the commonest operation in reverse engineering — was not covered, and it costs 10.1s for SIX renames on bash (1.7s each) because bump_names discards the segment's ListingModel and the reload re-walks it from the start. lg_split also rose to 3540ms: the split phase now runs after renames have thrown the listing away.","timestamp":1786073435987,"segment":6,"confidence":null,"asi":{"hypothesis":"keep probing unbenched features -- the last two probes each found a 10x","finding":"rename costs 1.7s per rename on bash (10.1s for six) and 106ms on echo. Program.bump_names() clears _listings, so the reload builds an empty ListingModel and ensure_ea walks the segment from its start to find the row the cursor was already on.","the_irony":"idatui/edit_ctl.py already documents that a rename cannot move a row: 'a rename or comment doesn't change how many rows anything takes' -- it restores the cursor by INDEX afterwards. It just throws away the walk that gives the index meaning.","fix_ready_and_verified":".auto/wip-renamekeep.patch. ListingModel.invalidate_text() keeps the walk and marks the rendered text stale; _ensure_text re-renders a 500-head block at a time, snapped out to whole ADDRESS groups (a function start emits three banner rows at the same ea, so an unsnapped block boundary refetches the group and never lines up). If a refetch does come back with a different head sequence it sets stale_structure and Program.listing() rebuilds, so a mis-routed structural edit degrades to today's behaviour instead of showing stale names.","measured":"bash, cursor at row 220036 of 228659: viewport back in 10.3ms instead of 6320ms (593x). A FULL re-read of every row is break-even with a rebuild (6.8s vs 6.9s), which is the right shape -- refreshing N heads costs what loading N heads costs.","equivalence":"/tmp/renameeq.py renames a function, snapshots every row (ea, kind, text, name) from the kept model, then rebuilds from scratch and compares: 0 mismatches over 5952 rows (echo), 28807 (ls_ttl) and 228659 (bash), three renames each.","one_open_question":"one full-suite run showed a follow_xrefs failure with the patch in; it passed 10/10 in isolation afterwards and 6/7 full runs green vs 6/6 without. Watch it.","next_action_hint":"apply the patch"}} diff --git a/idatui/domain.py b/idatui/domain.py index b5081cf..386b3e8 100644 --- a/idatui/domain.py +++ b/idatui/domain.py @@ -642,6 +642,11 @@ class ListingModel: """ PAGE = 500 # heads per server call (well under the tool's 2000 cap) + #: Heads refreshed together when a rename makes their text stale. One server + #: call per block, so a viewport costs one round trip rather than forty -- + #: and the same size as a load page, so refreshing everything costs about + #: what rebuilding everything would have. + TEXT_BLOCK = 500 def __init__(self, program: "Program", seg_start: int, seg_end: int, name: str | None = None): @@ -659,6 +664,19 @@ class ListingModel: # demand. _row_at[i] is the logical row where physical head i starts. self._row_at: list[int] = [] self._head_eas: list[int] = [] # parallel to _heads, for bisect + #: Which name generation each head's TEXT was rendered at, parallel to + #: _heads. A rename bumps :attr:`_text_gen`; the rows themselves stay + #: (their addresses and row numbers are unchanged) and are re-rendered a + #: block at a time when something asks for them. See invalidate_text. + self._head_gen: list[int] = [] + self._text_gen = 0 + #: Whether a rename has ever staled this model. Until one has, every + #: read takes exactly the path it always did. + self._renamed = False + #: Set if a text refresh came back with a different head sequence, which + #: means something DID move the walk. Program.listing() throws the model + #: away when it sees this, so the next read rebuilds from scratch. + self.stale_structure = False self._rows = 0 # total logical rows loaded self._ubytes: dict[int, bytes] = {} # lazily-read bytes for those rows self._next: int | None = seg_start # next address to fetch from @@ -739,6 +757,7 @@ class ListingModel: cur = payload.get("cursor", {}) if isinstance(payload, dict) else {} page = self._build_page(rows) with self._lock: + gen = self._text_gen for h in page: # Banner/label rows (function headers, separators, code labels) # are display-only; don't index them so navigation lands on the @@ -747,6 +766,7 @@ class ListingModel: self._by_ea.setdefault(h.ea, self._rows) self._row_at.append(self._rows) self._head_eas.append(h.ea) + self._head_gen.append(gen) self._heads.append(h) self._rows += self._span(h) nxt = cur.get("next") @@ -878,6 +898,78 @@ class ListingModel: def __len__(self) -> int: return self.loaded() + def invalidate_text(self) -> None: + """A rename changed how rows READ, not which rows exist. + + Item boundaries are untouched by a rename, so every row keeps its + address and its row number — which the edit path already relies on, since + it restores the cursor by INDEX afterwards. Dropping the whole model + instead means the next jump re-walks the segment from its start: 6.4 + seconds on bash's .text, after every single rename. + + So keep the walk and mark the rendered text stale; :meth:`_ensure_text` + re-renders a block at a time, and refuses to splice anything back if the + head sequence has moved under it (which a rename cannot do, but a + mis-routed structural edit could). + """ + with self._lock: + self._text_gen += 1 + self._renamed = True + + def _ensure_text(self, j0: int, j1: int) -> None: + """Re-render physical heads [j0, j1) if a rename staled them. + + The block is snapped out to whole ADDRESS groups. A function start emits + three banner rows and its code row at the same ea, and a labelled + instruction emits two -- so a block boundary that fell inside one of + those groups would refetch the whole group and never line up again. + """ + with self._lock: + gen = self._text_gen + n = len(self._heads) + j0 = max(j0, 0) + j1 = min(j1, n) + if j1 <= j0: + return + head_gen = self._head_gen + if all(head_gen[j] == gen for j in range(j0, j1)): + return + blk = self.TEXT_BLOCK + a = (j0 // blk) * blk + b = min(((j1 - 1) // blk + 1) * blk, n) + eas = self._head_eas + while a > 0 and eas[a - 1] == eas[a]: + a -= 1 + while b < n and eas[b - 1] == eas[b]: + b += 1 + last = self._heads[b - 1] + lo = eas[a] + hi = last.ea + max(last.size, 1) + want = [(h.ea, h.kind) for h in self._heads[a:b]] + try: + payload = self._prog.client.call( + "heads", addr=hex(lo), end=hex(hi), + count=min(len(want) + 64, 2000), annotate=True) + except Exception: # noqa: BLE001 -- keep the old text rather than blank + return + rows = payload.get("heads", []) if isinstance(payload, dict) else [] + page = self._build_page(rows)[:len(want)] + with self._lock: + if self._text_gen != gen or len(self._heads) < b: + return + if [(h.ea, h.kind) for h in page] != want: + # Something moved the walk, which a rename cannot do -- so this + # was not one. Say so and let Program.listing() rebuild, rather + # than sit here re-fetching a block that will never line up (and + # showing the old names while doing it). + self.stale_structure = True + for j in range(a, b): + self._head_gen[j] = gen + return + self._heads[a:b] = page + for j in range(a, b): + self._head_gen[j] = gen + def get(self, i: int) -> Head | None: with self._lock: if not (0 <= i < self._rows): @@ -885,8 +977,23 @@ class ListingModel: j, off = self._phys(i) if j < 0: return None - span = self._span(self._heads[j]) - h = self._heads[j] + stale = self._renamed and self._head_gen[j] != self._text_gen + if not stale: + span = self._span(self._heads[j]) + h = self._heads[j] + if stale: + # A rename staled this row's text; re-render its block (one call for + # the block around it, so a viewport costs one round trip). Only + # this path re-takes the lock -- the ordinary read stays atomic. + self._ensure_text(j, j + 1) + with self._lock: + if not (0 <= i < self._rows): + return None + j, off = self._phys(i) + if j < 0: + return None + span = self._span(self._heads[j]) + h = self._heads[j] # Synthesis reads bytes, so do it OUTSIDE the lock: an RPC under the # model lock deadlocks the page loader that is filling it. return self._row_head(j, off) if span > 1 else h @@ -895,6 +1002,16 @@ class ListingModel: """``count`` logical rows from ``start`` (synthesising undefined ones).""" self.ensure(start + count) with self._lock: + # _renamed stays set once a rename has happened; _ensure_text then + # does the precise, range-limited staleness check. Before the first + # rename this is one boolean and the read is exactly as it was. + dirty = self._renamed + if dirty: + j0 = max(self._phys(max(start, 0))[0], 0) + j1 = self._phys(max(min(self._rows, start + count) - 1, 0))[0] + 1 + if dirty: + self._ensure_text(j0, j1) + with self._lock: rows = min(self._rows, start + count) spans = [self._phys(i) for i in range(max(start, 0), max(rows, 0))] heads = self._heads @@ -1223,6 +1340,8 @@ class Program: start, end, name = seg with self._lock: m = self._listings.get(start) + if m is not None and m.stale_structure: + m = None # a refresh found the walk had moved; start over if m is None: m = ListingModel(self, start, end, name) self._listings[start] = m @@ -1406,15 +1525,24 @@ class Program: def bump_names(self) -> None: """Signal that symbol names changed (a rename). Disasm/listing names are - live in the IDB, so clearing the cached rows is enough for those; - decompilation is generation-checked and force-recompiled lazily.""" + live in the IDB, so the cached rows have to be re-rendered; decompilation + is generation-checked and force-recompiled lazily. + + The listing keeps its WALK. A rename cannot move an item boundary, so + every row keeps its address and its row number -- the edit path already + assumes exactly that, since it restores the cursor by index afterwards. + Dropping the segment model instead made the reload re-walk it from the + start, which is 6.4 seconds on bash after every rename. + """ with self._lock: self._name_gen += 1 models = list(self._disasm.values()) - self._listings.clear() # listing head rows cache names -> refetch + listings = list(self._listings.values()) self._pc_nums.clear() # a reformat moves every literal on its line for m in models: m.invalidate() + for lm in listings: + lm.invalidate_text() def bump_items(self) -> None: """Signal that item/function STRUCTURE changed (define code/data/func, |