diff options
| -rw-r--r-- | experiments/profile_remote.py | 1 | ||||
| -rw-r--r-- | idatui/app.py | 14 | ||||
| -rw-r--r-- | idatui/codemode_client.py | 2 | ||||
| -rw-r--r-- | idatui/domain.py | 53 | ||||
| -rw-r--r-- | idatui/remote_tools.py | 23 | ||||
| -rw-r--r-- | tests/test_scenarios.py | 57 |
6 files changed, 129 insertions, 21 deletions
diff --git a/experiments/profile_remote.py b/experiments/profile_remote.py index f35bac2..ca673db 100644 --- a/experiments/profile_remote.py +++ b/experiments/profile_remote.py @@ -47,6 +47,7 @@ CALLS = { # One full listing page, exactly as the background grower asks for it. "heads": 'heads(addr=a["addr"], count=500, annotate=True)', "heads_plain": 'heads(addr=a["addr"], count=500, annotate=False)', + "heads_skeleton": 'heads(addr=a["addr"], count=500, annotate=True, text=False)', "decompile": 'decompile(a["addr"])', "disasm": 'disasm(a["addr"], 500)', } diff --git a/idatui/app.py b/idatui/app.py index e193c65..a806d65 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -1262,13 +1262,23 @@ class ListingView(SearchMixin, NavMixin, ColumnCursor, ScrollView, can_focus=Tru @work(thread=True, exclusive=True, group="listing-grow") def _grow(self) -> None: """Stream the rest of the segment's heads in the background, growing the - virtual size as they land so the scrollbar/paging catch up.""" + virtual size as they land so the scrollbar/paging catch up. + + SKELETON pages: this loop only exists to find out how many rows the + segment has, and it used to render every one of them to do it -- 227k + rows for a 1.2MB bash, ~9s, essentially all never displayed. A skeleton + page has the same rows at the same addresses and no text, is 2.8x + cheaper and costs one round trip instead of two. The first read of one + materialises it through the same path a rename uses, so only what is + actually shown ever gets rendered. _prime (the viewport) still loads + real pages, so what you are looking at is never a skeleton. + """ model = self.model if model is None: return since = 0 while not model.complete: - if model.load_next_page() == 0: + if model.load_next_page(text=False) == 0: break if self.model is not model: # a new load() replaced us return diff --git a/idatui/codemode_client.py b/idatui/codemode_client.py index 5ee2780..d0d1937 100644 --- a/idatui/codemode_client.py +++ b/idatui/codemode_client.py @@ -1209,7 +1209,7 @@ _HEADS = _remote_op( 'heads(addr=a["addr"], count=int(a.get("count", 200)),' ' offset=int(a.get("offset", 0)), end=a.get("end", ""),' ' back=bool(a.get("back", False)), annotate=bool(a.get("annotate", False)),' - ' expect=a.get("expect", ""))') + ' expect=a.get("expect", ""), text=bool(a.get("text", True)))') # The graph view's only backend call. Blocks are address RANGES, never text: diff --git a/idatui/domain.py b/idatui/domain.py index d65c473..903611f 100644 --- a/idatui/domain.py +++ b/idatui/domain.py @@ -683,6 +683,9 @@ class ListingModel: """ PAGE = 500 # viewport-scale heads per Code Mode execution + #: Generation marker for a skeleton (text-less) page. Never equals a real + #: _text_gen, which counts up from 0, so such a page always reads as stale. + _SKELETON_GEN = -1 def __init__(self, program: "Program", seg_start: int, seg_end: int, name: str | None = None): @@ -709,6 +712,10 @@ class ListingModel: #: Whether a rename has ever staled this model. Until one has, every #: read takes exactly the path it always did. self._renamed = False + #: Whether any page was loaded as a text-less skeleton. Same effect as + #: _renamed -- reads have to check the per-head generation -- so the two + #: are ORed at every gate rather than duplicating the machinery. + self._skeleton = False #: One entry per loaded PAGE: where its heads start, the address it was #: fetched from, the digest it came back with, and how many rows it #: held. A stale-text refresh re-asks for exactly that page, so it can @@ -735,7 +742,7 @@ class ListingModel: # containing a huge coalesced undefined run doesn't pull megabytes. _OP_SPAN_CAP = 1 << 16 - def _build_page(self, rows: list) -> list[Head]: + def _build_page(self, rows: list, raw: bool = True) -> 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. @@ -753,7 +760,10 @@ class ListingModel: lo = ea hi = ea + int(r["size"]) data = None - if 0 <= lo < hi and hi - lo <= self._OP_SPAN_CAP: + # A skeleton page shows no text, so it needs no opcode bytes -- and + # skipping them drops the SECOND round trip a page costs (heads is + # always followed by a bulk read_raw over the code extent). + if raw and 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 @@ -782,26 +792,42 @@ class ListingModel: with self._lock: return self._max_raw - def load_next_page(self) -> int: - """Load one more page of heads; returns how many were added.""" - return self._load_next_page() + def load_next_page(self, text: bool = True) -> int: + """Load one more page of heads; returns how many were added. - def _load_next_page(self) -> int: + ``text=False`` loads a SKELETON page: the same rows at the same + addresses with the same sizes and kinds, but no rendered disassembly + and no opcode bytes -- 2.8x cheaper, and one round trip instead of two. + + That is all the background grower needs. It exists to discover how many + rows the segment has so the scrollbar and paging are right, and it + renders 227k rows of a 1.2MB bash to do it, essentially all of which are + never looked at. A skeleton page is marked text-stale, so the FIRST read + of one goes through exactly the same ``_ensure_text`` path a rename uses + and materialises it, one page per round trip, only for what is shown. + """ + return self._load_next_page(text) + + def _load_next_page(self, text: bool = True) -> int: with self._load_lock: - return self._load_next_page_locked() + return self._load_next_page_locked(text) - def _load_next_page_locked(self) -> int: + def _load_next_page_locked(self, text: bool = True) -> int: with self._lock: if self._done or self._next is None: return 0 frm = self._next payload = self._prog.client.invoke( - "heads", addr=hex(frm), count=self.PAGE, annotate=True) + "heads", addr=hex(frm), count=self.PAGE, annotate=True, text=text) rows = payload.get("heads", []) if isinstance(payload, dict) else [] cur = payload.get("cursor", {}) if isinstance(payload, dict) else {} - page = self._build_page(rows) + page = self._build_page(rows, raw=text) with self._lock: - gen = self._text_gen + # A sentinel generation no _text_gen can ever equal, so the page + # reads as stale until something asks for it and refreshes it. + gen = self._text_gen if text else self._SKELETON_GEN + if not text: + self._skeleton = True self._page_head.append(len(self._heads)) self._page_addr.append(frm) self._page_digest.append(payload.get("digest") @@ -1104,7 +1130,8 @@ class ListingModel: j, off = self._phys(i) if j < 0: return None - stale = self._renamed and self._head_gen[j] != self._text_gen + stale = ((self._renamed or self._skeleton) + and self._head_gen[j] != self._text_gen) if not stale: span = self._span(self._heads[j]) h = self._heads[j] @@ -1132,7 +1159,7 @@ class ListingModel: # _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 + dirty = self._renamed or self._skeleton if dirty: j0 = max(self._phys(max(start, 0))[0], 0) j1 = self._phys(max(min(self._rows, start + count) - 1, 0))[0] + 1 diff --git a/idatui/remote_tools.py b/idatui/remote_tools.py index dca8f14..1f66193 100644 --- a/idatui/remote_tools.py +++ b/idatui/remote_tools.py @@ -102,13 +102,20 @@ def compact_whitespace(line: str) -> str: # used to sit here, shadowed by the real ones below. If you find one again: # keep the copy carrying @lru_cache. Deleting that one instead is a silent # ~2.7x regression on every listing row (10.4us -> 3.9us is the cache). -def _idatui_head_row(ea, flags=None): +def _idatui_head_row(ea, flags=None, text=True): """One flat-listing row for the head at ``ea``: kind (code/data/unknown), byte size, rendered text, and any symbol name. ``flags`` lets a caller that already asked for them say so -- the walk in ``heads`` used to fetch them three times per head (here, in _is_unknown from _advance, and again from _rows_for). + + ``text=False`` builds a SKELETON row: address, kind, size and name, but no + rendered text and no colour spans. generate_disasm_line is 22x the cost of + the walk around it, and a caller that only needs to know how many rows a + segment has -- which is what sizing the scrollbar needs -- should not pay + it. The row COUNT and the addresses are identical either way, which is what + makes a skeleton page swappable for a real one later. """ f = ida_bytes.get_flags(ea) if flags is None else flags @@ -118,8 +125,11 @@ def _idatui_head_row(ea, flags=None): kind = "data" else: kind = "unknown" - line = ida_lines.generate_disasm_line(ea, 0) - text, spans, ops = _idatui_line_parts(line) if line else ("", None, None) + if text: + line = ida_lines.generate_disasm_line(ea, 0) + text, spans, ops = _idatui_line_parts(line) if line else ("", None, None) + else: + text, spans, ops = "", None, None row = { "ea": hex(ea), "kind": kind, @@ -492,6 +502,7 @@ def heads( back: Annotated[bool, "Walk backwards: return the count heads ENDING just before addr, in forward order"] = False, annotate: Annotated[bool, "Emit IDA-style function boundary banner rows (kind sep/funchdr)"] = False, expect: Annotated[str, "Digest a caller already holds: the rows are omitted when they still hash to it"] = "", + text: Annotated[bool, "Render each row's disassembly text (default true). False = a skeleton page: same rows, same addresses, no text"] = True, ) -> dict: """Walk item heads from ``addr`` as a flat listing: every head is rendered (code OR data OR undefined) via generate_disasm_line and stepped with @@ -578,7 +589,7 @@ def heads( out = [] if at_start: out.extend(_idatui_func_header_rows(e)) - row = _idatui_head_row(e, f) + row = _idatui_head_row(e, f, text) if at_start: row = dict(row) row["name"] = None # the name is shown on the proc header line @@ -612,7 +623,9 @@ def heads( rows.extend(_rows_for(ea, f)) # a struct head expands into member rows ea = _advance(ea, f) cursor = {"next": hex(ea)} if more else {"done": True} - dig = _idatui_rows_digest(rows) + # A skeleton page has no text to go stale, so there is nothing to digest -- + # and the digest is only ever used to skip re-sending text. + dig = _idatui_rows_digest(rows) if text else None out = {"addr": str(addr), "cursor": cursor, "digest": dig, "count": len(rows)} # ``expect`` says "I already hold a page that hashed to this". The rows are # built either way -- generate_disasm_line is the floor and there is no way diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index 038cac5..780459a 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -412,6 +412,63 @@ async def s_auto_land(c: Ctx): c.check("auto-land is idempotent (guarded)", app._cur is prev) +@scenario("skeleton_pages") +async def s_skeleton_pages(c: Ctx): + """The background grower loads text-less pages; reading one must fill it in. + + _grow streams the whole segment only to learn how many rows it has, so it + asks for skeleton pages (same rows, same addresses, no rendered text) -- + 3x cheaper and one round trip instead of two. The first read of such a page + has to materialise it through the same path a rename uses. + + The failure mode if that path breaks is BLANK ROWS deep in the listing, not + an exception, and nothing else in this suite scrolls far enough to see it: + _prime renders the first ~1000 rows for real, so a test that only pages down + a few screens passes against a completely broken implementation. + """ + app = c.app + await c.open_biggest("listing") + lv = app.query_one(ListingView) + model = lv.model + if model is None: + c.check("listing model exists", False) + return + # Let the grower finish so the tail of the segment is definitely skeleton. + await c.wait(lambda: model.complete, 30) + c.check("the grower completes", model.complete, f"rows={len(model)}") + if not model.complete or len(model) < 1200: + # A target smaller than _prime's horizon has no skeleton pages at all, + # so there is nothing to check rather than something broken. + c.check("segment is big enough to have skeleton pages", True, + f"skipped: only {len(model)} rows, _prime renders ~1000") + return + c.check("pages were loaded as skeletons", model._skeleton is True) + + # Well past _prime's horizon, and the very last row. + deep = max(1200, len(model) - 40) + for row in (1200, len(model) // 2, deep): + h = model.get(row) + c.check(f"row {row} of a skeleton page has real text", + h is not None and bool((h.text or "").strip()), + f"ea={getattr(h, 'ea', None)} text={getattr(h, 'text', None)!r}") + + # And through the render path the user actually sees, not just the model. + lv.cursor = deep + lv.refresh() + await c.pause(0.1) + painted = lv._line_plain(deep) + c.check("a deep row RENDERS with text", + bool(painted and painted.strip()), f"painted={painted!r}") + + # Materialising must not change the row count or move any address: the + # skeleton's structure is what the scrollbar was sized from. + before = len(model) + model.get(deep) + c.check("materialising a page does not change the row count", + len(model) == before, f"{before} -> {len(model)}") + c.check("the walk was not disturbed", not model.stale_structure) + + @scenario("palette_paging") async def s_palette_paging(c: Ctx): """PgUp/PgDn move the palette list by a viewport, with the Input focused. |