diff options
| -rw-r--r-- | .auto/log.jsonl | 1 | ||||
| -rw-r--r-- | server/patch_server.py | 102 |
2 files changed, 57 insertions, 46 deletions
diff --git a/.auto/log.jsonl b/.auto/log.jsonl index ebef486..55322e0 100644 --- a/.auto/log.jsonl +++ b/.auto/log.jsonl @@ -1,3 +1,4 @@ {"type":"config","name":"ida-tui performance: cut the latency of navigation, listing, decomp, graph and search","metricName":"total_ms","metricUnit":"ms","bestDirection":"lower"} {"run":1,"commit":"2910c93","metric":46572.1,"metrics":{"lg_boot_ms":863.5,"lg_decomp_ms":2881.3,"lg_graph_ms":951.7,"lg_hex_ms":900.9,"lg_index_ms":219.3,"lg_listing_cold_ms":434.4,"lg_listing_warm_ms":552.3,"lg_nav_ms":29106.9,"lg_palette_ms":4.8,"lg_render_ms":231.2,"lg_search_ms":5590.1,"pure_graph_ms":236.2,"sm_boot_ms":539.4,"sm_decomp_ms":621.4,"sm_graph_ms":702,"sm_hex_ms":850.5,"sm_index_ms":0,"sm_listing_cold_ms":270.8,"sm_listing_warm_ms":270.6,"sm_nav_ms":875.1,"sm_palette_ms":0.3,"sm_render_ms":272.3,"sm_search_ms":197.1,"fails":0},"status":"checks_failed","description":"Baseline run of the new bench harness. Benchmark clean (fails=0) but the pilot scenario suite reported 300 passed / 1 failed with ZERO code changes -> flaky, and checks.sh printed the tail instead of the FAIL line so the name is unknown.","timestamp":1786058161497,"segment":0,"confidence":null,"asi":{"hypothesis":"establish a baseline for total_ms","bottleneck":"lg_nav_ms=29107 is 62% of total_ms; lg_nav_worst_ms=28274 is ONE cold jump to a high address in bash. ListingModel.ensure_ea walks the segment forward in 500-head pages from seg_start, so landing near the end of a 224k-row listing costs ~450 sequential worker round trips.","second_bottleneck":"lg_search_ms=5590 (91783 hits over the whole segment)","cheap_phases":"palette/index/render/pure_graph are all <600ms; not where the time is","rollback_reason":"checks.sh flagged 1 scenario failure with no code change (flake)","next_action_hint":"make checks.sh print the FAIL line name on non-zero exit, re-run baseline, then attack ListingModel address->row lookup (needs a backend primitive in server/patch_server.py: heads walking anchored at an address, or a segment head-index built in one call)"}} {"run":2,"commit":"a3f3400","metric":46685.5,"metrics":{"lg_boot_ms":885.7,"lg_decomp_ms":2871.8,"lg_graph_ms":930.9,"lg_hex_ms":947.3,"lg_index_ms":207.8,"lg_listing_cold_ms":564.3,"lg_listing_warm_ms":454.7,"lg_nav_ms":29018.5,"lg_palette_ms":4.6,"lg_render_ms":223.6,"lg_search_ms":5448.7,"pure_graph_ms":535.4,"sm_boot_ms":537.9,"sm_decomp_ms":643.8,"sm_graph_ms":680.4,"sm_hex_ms":854.9,"sm_index_ms":0,"sm_listing_cold_ms":265.5,"sm_listing_warm_ms":263.7,"sm_nav_ms":887.2,"sm_palette_ms":0.3,"sm_render_ms":264.1,"sm_search_ms":194.5,"fails":0},"status":"discard","description":"Baseline re-run with the fixed checks gate. Checks pass; total_ms reproduces to within 0.24% of run #1 (46572 -> 46686), so the noise floor is ~115ms on a 46.6s metric.","timestamp":1786058370297,"segment":0,"confidence":null,"asi":{"hypothesis":"confirm the baseline is reproducible and the checks gate is green","noise_floor_ms":115,"reproducibility":"run1 46572 / run2 46686 -> 0.24% spread; pure_graph_ms is the jumpiest single phase (236 -> 535, it is CPU-only and gets descheduled)","checks":"flaky-scenario retry logic works; suite green on a clean tree","next_action_hint":"attack ListingModel.ensure_ea / the heads tool: 29s of 46.6s is one cold address->row walk over bash"}} +{"run":3,"commit":"93240e2","metric":26923.9,"metrics":{"lg_boot_ms":762.2,"lg_decomp_ms":2631.7,"lg_graph_ms":941.8,"lg_hex_ms":1052,"lg_index_ms":67.2,"lg_listing_cold_ms":440.6,"lg_listing_warm_ms":530.5,"lg_nav_ms":10598.3,"lg_palette_ms":4.6,"lg_render_ms":227.8,"lg_search_ms":5265.5,"pure_graph_ms":237.9,"sm_boot_ms":535.3,"sm_decomp_ms":631.8,"sm_graph_ms":702.1,"sm_hex_ms":841.1,"sm_index_ms":0,"sm_listing_cold_ms":268.2,"sm_listing_warm_ms":269.4,"sm_nav_ms":443.6,"sm_palette_ms":0.3,"sm_render_ms":277.8,"sm_search_ms":194.4,"fails":0},"status":"keep","description":"Stop ida-pro-mcp installing a sys.setprofile hook around every tool call. Its deadline mechanism profiles every python call/return so a pure-python tool loop can be interrupted; our tools are call-heavy, so it taxed the whole backend 3.3x. Worker now sets IDA_MCP_TOOL_TIMEOUT_SEC=0 and arms the deadline itself with one polling watchdog thread + ida_kernwin.set_cancelled() (the half that actually frees the IDA main thread). Also rewrote _idatui_spans to jump between colour tags instead of walking characters (byte-identical over 258k real lines).","timestamp":1786059117542,"segment":0,"confidence":173.264550264548,"asi":{"hypothesis":"the heads tool is not IDA-bound; the ida-pro-mcp sync wrapper's sys.setprofile deadline is the tax","evidence":"in-process A/B on targets/bash: heads(count=500,annotate) 92.2us/row with IDA_MCP_TOOL_TIMEOUT_SEC unset vs 27.5us/row with it 0; domain ListingModel._load_next_page 116 -> 37 us/row","where_it_lives":"site-packages/ida_pro_mcp/ida_mcp/sync.py sync_wrapper() installs profilefunc via sys.setprofile whenever timeout>0; default _DEFAULT_TOOL_TIMEOUT_SEC=60","what_we_kept":"the native half: threading watchdog + ida_kernwin.set_cancelled(), which is what decompile/auto_wait/find_bytes actually poll. Only the interruption of a pure-python loop inside a tool body is gone, and every idatui tool loop is bounded by its count argument (<=2000).","watchdog_design":"polling at 0.25s instead of signalling, because arm/disarm is on the path of EVERY worker call (a scroll is dozens); arming is two attribute writes + clr_cancelled()","spans_rewrite":"_idatui_spans now re.finditer's over the 3 control chars instead of walking characters. Verified byte-identical vs HEAD over 258k real disasm lines across echo/ls_ttl/bash/libcrypto (.auto/diff_spans.py). Standalone it measured 12.5 -> 13.6 us/line i.e. slightly SLOWER, so it is probably a small net loss that the setprofile win is hiding -- A/B it on its own next.","gains":"total 46572 -> 26924 (-42%); lg_nav 29107 -> 10598; lg_nav_worst 28274 -> 10170; sm_nav_worst 656 -> 224; lg_index 219 -> 67","work_unchanged":"nav_rows/decomp_ok/graph_ok/search_hits/render_cells all identical -> the speedup is not less work","next_action_hint":"1) A/B the _idatui_spans rewrite alone (suspect regression). 2) lg_nav is still 10.6s: ListingModel.ensure_ea is O(distance) at ~34us/row; a skeleton (ea,size,kind only) walk or a bigger PAGE would cut it further. 3) lg_search_ms 5265 is now the #2 term."}} diff --git a/server/patch_server.py b/server/patch_server.py index b806be2..b75b120 100644 --- a/server/patch_server.py +++ b/server/patch_server.py @@ -364,8 +364,7 @@ def _idatui_tag_map(): _IDATUI_TAGS = None _IDATUI_OPND_TAGS = None -_IDATUI_CTL = None # re: the three control characters a tagged line can hold -_IDATUI_WS = None # re: a run of whitespace, exactly what str.isspace() calls one +_IDATUI_CTL = None # re: a tag = one of three control chars plus its argument def _idatui_opnd_tag_map(): @@ -395,7 +394,7 @@ def _idatui_spans(line): Unknown tags become 'text' rather than being dropped: a processor module can emit a colour we don't classify, and losing the characters would corrupt the line.""" - global _IDATUI_TAGS, _IDATUI_OPND_TAGS, _IDATUI_CTL, _IDATUI_WS + global _IDATUI_TAGS, _IDATUI_OPND_TAGS, _IDATUI_CTL import ida_lines if _IDATUI_TAGS is None: _IDATUI_TAGS = _idatui_tag_map() @@ -403,48 +402,54 @@ def _idatui_spans(line): _IDATUI_OPND_TAGS = _idatui_opnd_tag_map() if _IDATUI_CTL is None: import re as _re - _IDATUI_CTL = _re.compile("[\\x01\\x02\\x03]") - # str.isspace() is true for \\x1c-\\x1f and \\x85 as well as the \\s - # class, so spell those out: this substitution has to agree with the - # plain-text collapse character for character (checked over every - # codepoint) or the row silently loses its highlighting. - _IDATUI_WS = _re.compile("[\\\\s\\x1c\\x1d\\x1e\\x1f\\x85]+") - tags, opnds, ctl = _IDATUI_TAGS, _IDATUI_OPND_TAGS, _IDATUI_CTL + # One capturing split gives [text, tag, text, tag, ..., text] in a + # single C pass. A per-character python loop over the line used to be + # 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])") + tags, opnds = _IDATUI_TAGS, _IDATUI_OPND_TAGS on, off, esc = "\x01", "\x02", "\x03" addr_tag = chr(getattr(ida_lines, "COLOR_ADDR", 0x28)) addr_len = int(getattr(ida_lines, "COLOR_ADDR_SIZE", 16)) - # Jump between control characters and take the text in between as one slice. - # A per-character loop here was 68% of the whole `heads` tool: a disasm line - # is ~50 characters but only ~15 tags, and everything between two tags is - # already exactly one span's worth of text. - spans, stack, buf = [], [], [] # stack entries: (kind, operand index|None) + parts = _IDATUI_CTL.split(line) + spans, stack = [], [] # stack entries: (kind, operand index|None) kind, opnd = "text", None # state the current run of text belongs to - i, n = 0, len(line) - for m in ctl.finditer(line): - j = m.start() - if j < i: # inside an address payload / after an esc - continue - if j + 1 >= n: # a trailing control char is literal text + pend = "" + skip = 0 # characters of an address payload still due + i, n = 0, len(parts) + while i < n: + txt = parts[i] + i += 1 + if skip: + if len(txt) <= skip: + skip -= len(txt) + txt = "" + else: + txt = txt[skip:] + skip = 0 + if txt: + pend += txt + if i >= n: break - ch = line[j] + pair = parts[i] + i += 1 + if skip: # a tag INSIDE an address payload: 2 chars + skip = skip - 2 if skip > 2 else 0 + continue + ch = pair[0] if ch == esc: # escaped literal: keep the char it guards - buf.append(line[i:j]) - buf.append(line[j + 1]) - i = j + 2 + pend += pair[1] continue - tag = line[j + 1] + tag = pair[1] if ch == on and tag == addr_tag: # An embedded target address, not display text: 16 hex digits that # must not reach the screen. Deliberately NOT a span boundary. - buf.append(line[i:j]) - i = j + 2 + addr_len + skip = addr_len continue - buf.append(line[i:j]) - i = j + 2 - txt = "".join(buf) - if txt: - spans.append([kind, txt, opnd]) - del buf[:] + if pend: + spans.append([kind, pend, opnd]) + pend = "" if ch == on: stack.append((kind, opnd)) kind = tags.get(tag, "text") @@ -455,24 +460,29 @@ def _idatui_spans(line): kind, opnd = stack.pop() else: kind, opnd = "text", None - if i < n: - buf.append(line[i:]) - txt = "".join(buf) - if txt: - spans.append([kind, txt, opnd]) + if pend: + spans.append([kind, pend, opnd]) # Collapse IDA's column padding EXACTLY as the plain text does. A run of # spaces can straddle two spans, so the leading space of a span is dropped # when the previous one ended in space — otherwise the spans and `text` # disagree about the line and the row silently loses its highlighting. + # ``" ".join(txt.split())`` splits on exactly what str.isspace() calls + # whitespace, which is what the character walk this replaces tested. out, prev_space = [], False - ws = _IDATUI_WS for kind, txt, opnd in spans: - acc = ws.sub(" ", txt) - if prev_space and acc[:1] == " ": - acc = acc[1:] - if acc: - prev_space = acc[-1] == " " - out.append([kind, acc, opnd]) + core = " ".join(txt.split()) + if not core: # the span is nothing but padding + if not prev_space: + prev_space = True + out.append([kind, " ", opnd]) + continue + acc = core + if txt[0].isspace() and not prev_space: + acc = " " + acc + if txt[-1].isspace(): + acc += " " + prev_space = acc[-1] == " " + out.append([kind, acc, opnd]) while out and out[0][1] == " ": out.pop(0) while out and out[-1][1] == " ": |