diff options
| -rw-r--r-- | .auto/ideas.md | 99 | ||||
| -rw-r--r-- | .auto/log.jsonl | 3 | ||||
| -rw-r--r-- | .auto/parked/check_decomp.py | 110 | ||||
| -rw-r--r-- | .auto/parked/fast_decompile.patch | 122 | ||||
| -rw-r--r-- | .auto/parked/fast_pc_nums.patch | 52 |
5 files changed, 386 insertions, 0 deletions
diff --git a/.auto/ideas.md b/.auto/ideas.md index 9586574..133993c 100644 --- a/.auto/ideas.md +++ b/.auto/ideas.md @@ -230,3 +230,102 @@ come from the same ctree. `Program.decompile` calls the `force_recompile` tool `ida_hexrays.decompile()` repopulates the cache and `decomp_map` then hits it. `/tmp/mapalign.py` appeared to show a mismatch only because the probe itself used `DECOMP_NO_CACHE`, which the app never does. + +## PARKED: a 3.6x faster decompile_function_safe (measured, byte-identical, discarded on the metric) + +ida-pro-mcp's `decompile_function_safe` — the function that produces the +pseudocode the pane shows — has the **same three faults** that were fixed in +`decomp_map`: + +* it allocates **three** `ctree_item_t` SWIG objects per pseudocode line, and + `_head` and `_tail` are never read (`get_line_item` takes None for both); +* it calls `dstr()` per line to recover the `/*0xEA*/` marker — 24 µs a call — + where consecutive lines of a multi-line expression report the same ctree item, + so memoising by `obj_id` (unique within a cfunc) skips most of them. + +Measured, on a **warm** cfunc (so this is pure post-processing, no Hex-Rays): + +| workload | before | after | +|---|---|---| +| bash's 8 largest, 18 991 lines (`/tmp/decprof.py`) | 2 302 ms (121.2 µs/line) | 429 ms (22.6 µs/line) | +| the same through the real worker (`/tmp/verifybind.py`) | 2 695 ms | 746 ms | +| echo's 12 largest, 3 298 lines (`/tmp/splitprof.py`) | 219 ms | 120 ms (30 µs/line) | + +Byte-identical: `check_decomp.py` runs both implementations against the same +cfunc with `include_addresses` both ways — **128/128 echo and 372/372 ls_ttl**. +It is a real gate: keying the memo on `it.op` instead of `it.obj_id` fails 69 of +128. + +**Discarded anyway**, because it does not move `total_ms`. Three runs with it +(25 367 / 25 058 / 25 171) against three without (24 514 / 25 122 / 25 783) — +the means are 25 199 vs 25 140, i.e. indistinguishable. The reason is in the +third row of the table: the saving is 98 µs/line on bash's *largest* functions +but only 30 µs/line on small ones, and the bench's fixed set averages 286 lines +a function. Expected effect ~150–250 ms against a run-to-run spread of ±400–600 +on this box. + +**It is still a real win for the operation a user waits on** — an F5 on a +2 374-line function drops 287 ms → 54 ms of post-processing — so it is parked +rather than deleted: + +* `.auto/parked/fast_decompile.patch` (applies to `server/patch_server.py` and + `idatui/worker.py`) +* `.auto/parked/check_decomp.py` (re-wire into `checks.sh` if the patch is + re-applied; it will crash if run without it, since it slices the function out + of `BODY`) + +Re-apply it if the benchmark ever decompiles large functions, or if the goal +moves from total session time to per-operation latency. Do **not** re-shape the +bench's fixture set to make this win visible — that would be fitting the +benchmark to the change. + +## PARKED: pc_nums, the third instance of the same bug (2.03x, byte-identical) + +`_idatui_pc_nums` allocated **three `ctree_item_t` SWIG objects per candidate +column** — inside a scan that probes every literal-looking character of every +pseudocode line. `a`–`f` are hex digits, so `a1`, `v6` and `sub_1F4C0` all +qualify and most columns of a line get probed. `head` and `tail` were never +read. This is the same fault as `decomp_map`'s sweep and +`decompile_function_safe`'s loop — **three instances of one bug**. + +It is on the F5 path: `app.py:_load_decomp` fetches `pc_nums` after every +successful decompile so the view can mark the literal under the cursor without a +round trip per keypress. + +Measured warm (Hex-Rays already cached), bash's 8 largest, 18 991 lines: +**1 247 ms → 614 ms (2.03x), 66 → 32 µs/line**, with the literal count identical +(7 015). Also stopped `tag_remove` running twice over every line (`pc_nums` and +`_idatui_pc_nums` each called it); worth ~nothing on its own but it is strictly +less work. + +Equivalence: the tool's whole output dumped per function and compared across +revisions (`/tmp/pcnumdump.py`, cross-process — an in-process differential +**segfaults**, two SWIG item objects over one cfunc). echo: 128 functions, +1 463 literals, **0 mismatches**. `tests/test_scenarios.py` also drives literal +cycling through these exact column extents. + +Parked at `.auto/parked/fast_pc_nums.patch` for the same reason as the decompile +patch: real work removed, but ~170 ms against a ±500 ms run-to-run spread. + +## Both parked patches, measured together + +Applied together (they are both on the F5 path) over four runs: 25 367 / 25 058 / +25 171 / 24 585, against three without: 24 514 / 25 122 / 25 783. Means **25 045 +with, 25 140 without** — a 95 ms edge inside a 500 ms spread, i.e. still not +resolvable. Discarded on the metric, kept on disk. + +Apply both if the goal moves to per-operation latency: an F5 on a 2 374-line +function loses ~233 ms of text post-processing and halves its `pc_nums` cost. + +## A 27 283 ms outlier, and how to recognise one + +One run came back at 27 283 (against ~24 500) with `lg_split` 2 200 → 6 917 and +`lg_search` 3 441 → 840. It was NOT the change under test: the **work counters +moved with it** — `lg_decomp_lines` 3 436 → 3 233, `lg_split_mapped_lines` +2 070 → 1 993, `lg_search_hits` 91 783 → 92 733. Hex-Rays decompiled bash +differently that run (the drift documented above), which changed how much of the +post-rename re-render the split phase absorbed before search got to it. The next +run reproduced 24 585 with every counter back to its usual value. + +**The NOTES counters are what tell an outlier from a regression.** A real +regression moves the time and leaves the work alone. diff --git a/.auto/log.jsonl b/.auto/log.jsonl index 97b8104..1b24d9e 100644 --- a/.auto/log.jsonl +++ b/.auto/log.jsonl @@ -48,3 +48,6 @@ {"run":41,"commit":"7e4f086","metric":25814,"metrics":{"lg_boot_ms":714.9,"lg_decomp_ms":2376.2,"lg_graph_ms":948.8,"lg_hex_ms":458.6,"lg_index_ms":69.3,"lg_listing_cold_ms":448.5,"lg_listing_warm_ms":482.1,"lg_nav_ms":6808,"lg_palette_ms":4.7,"lg_rename_ms":752,"lg_render_ms":219.4,"lg_search_ms":4008.3,"lg_split_ms":2275.5,"pure_graph_ms":225.2,"sm_boot_ms":456.1,"sm_decomp_ms":1292.4,"sm_graph_ms":795.8,"sm_hex_ms":475.2,"sm_index_ms":2.5,"sm_listing_cold_ms":269.1,"sm_listing_warm_ms":272.2,"sm_nav_ms":320.2,"sm_palette_ms":0.3,"sm_rename_ms":446.3,"sm_render_ms":264.8,"sm_search_ms":60.6,"sm_split_ms":1366.9,"fails":0},"status":"keep","description":"Three redundancies in the heads walk: item flags were fetched three times per head (row builder, _is_unknown via _advance, and _rows_for), get_func was called per head where a head is nearly always in the same function as the one before it, and the page digest rebuilt a tuple-of-tuples per row where one spans list is shared by ~45% of them. Cold heads 18.62 -> 17.77 us/row, warm 11.53 -> 10.87.","timestamp":1786078727527,"segment":6,"confidence":7.765877831715214,"asi":{"gains":"total 26492 (previous best) -> 25814 (-2.6%); lg_split 2659 -> 2276; sm_nav_worst 158 -> 151. Microbenchmarked rather than trusted to total_ms, since ~0.8us/row over ~500k row-renders is 0.4s and the box's spread is ~500ms: cold heads 18.62 -> 17.77 us/row, warm 11.53 -> 10.87, digest 0.75 -> 0.62.","equivalence":"/tmp/headsdump.py runs the tool under both code versions in separate processes and compares whole payloads: 109 004 rows over 294 payloads (bash + ls_ttl, BOTH annotate modes, plus the offset/end/back/count variants that share the walk) -- 0 mismatches. diff_spans still byte-identical vs 2b0ae8d on three binaries. 830-check gate, check_search and check_rename green.","get_func_cache_was_validated_not_assumed":"I had rejected this earlier on a hunch about IDA function CHUNKS -- a tail chunk of B inside A's [start_ea, end_ea) would be misattributed. /tmp/funccache.py walks every head of bash/ls_ttl/echo comparing the cached answer against get_func: 437 324 heads, 0 disagreements. Hit rate is only ~50% across all segments (a head outside any function cannot be cached by range) but much higher inside .text, which is what the listing walks.","TWO_SCHEMES_MEASURED_AND_REJECTED_FIRST":"before micro-optimising I tested the two big parked ideas, both of which would have made a rename nearly free. Listing rows predicted from xrefs_to + function extent: ls_ttl had 1 of 54 changed rows uncovered, 'lea rcx, unk_1D7A0' -> 'byte_1D7A0', which the rename did not cause -- IDA's own analysis defined that byte. Decompilations predicted from 'the old name appears in the cached text': 16 misses over 4 renames, EVERY one Hex-Rays type inference moving (unsigned int a4 -> int a4) in functions unrelated to the rename. Both recorded in .auto/ideas.md.","generalised_lesson":"predicting the effect of an edit on a database that has its own opinions is unsound. Verify instead -- which is exactly why heads(digest=True) works: it asks what a row renders as NOW, not what should have changed. This probably also explains the lg_decomp_lines drift blamed on CPU starvation in v5 #4.","things_checked_and_found_not_worth_it":{"digest_on_the_normal_load_path":"suspected a regression from v7 #10; it is 0.62-0.75 us/row, not the 1.9 the noisy call timing suggested","folding_the_opcode_byte_read_into_heads":"the extra read_bytes round trip is 0.35 us/row, ~81ms over a whole bash segment","pipelining_client_parsing_with_worker_compute":"blocked -- cursor.next is inside the pickle, so the next request cannot be issued before unpickling, and _build_page itself needs the socket for opcode bytes"},"state":"generate_disasm_line is now 6.09 of the 10.87 us/row warm cost (56%) and is IDA's. What is left of ours is ~4.8 us/row spread thin across a dozen places.","next_action_hint":"lg_nav 6808 (26%) and lg_search 4008 (16%) are both at their per-row floors now. The unexplored areas are the ones .auto/ideas.md lists as blocked or unmeasured: the trace memory index (needs test_trace_vs_tenet runnable), the streaming-responsiveness question, and the features the bench still does not drive (xrefs dialog, strings browser, struct editor, make-code edits, history, traces, RPC)."}} {"run":42,"commit":"16318e4","metric":25783,"metrics":{"lg_boot_ms":777.9,"lg_decomp_ms":2381.9,"lg_graph_ms":1209.5,"lg_hex_ms":450,"lg_index_ms":68,"lg_listing_cold_ms":434.6,"lg_listing_warm_ms":405.2,"lg_nav_ms":6801.2,"lg_palette_ms":4.7,"lg_rename_ms":744.7,"lg_render_ms":222.4,"lg_search_ms":3885.6,"lg_split_ms":2261.4,"pure_graph_ms":216.4,"sm_boot_ms":463.6,"sm_decomp_ms":1304.6,"sm_graph_ms":740.9,"sm_hex_ms":438.4,"sm_index_ms":2.4,"sm_listing_cold_ms":270.3,"sm_listing_warm_ms":267.2,"sm_nav_ms":312.4,"sm_palette_ms":0.3,"sm_rename_ms":415.8,"sm_render_ms":257.7,"sm_search_ms":60.8,"sm_split_ms":1385.2,"fails":0},"status":"keep","description":"An item edit (c/d/u/p) keeps the listing's walk in front of it instead of discarding the model. bump_items now takes the edited address; rows before an edit keep their addresses and their row numbers, so only the pages from the edit onward are re-walked. Getting the listing back after undefining at the cursor on bash: 4890ms -> 19ms (257x). Adds .auto/check_edit.py to the gate. total_ms is flat — the bench has no item-edit phase, and the one I wrote hangs (reverted, cause recorded).","timestamp":1786082485850,"segment":6,"confidence":5.464825819307547,"asi":{"how_it_was_found":"kept probing features the bench does not drive -- the pattern that produced the flowchart hull, decomp_map and the rename walk. /tmp/itemedit.py: the undefine tool call is 1ms and getting the listing back is 4890ms, because bump_items cleared _listings and the reload re-walked the segment. Same bug as the rename one, in the sibling path.","measured":"edit at the cursor (96% into bash's .text): 4890 -> 19ms. Edit early in the segment: 5130 -> 4542ms, which is the honest case -- everything after an edit really does have to be re-walked.","why_it_is_safe":"truncate_from drops two pages rather than one (undefining can coalesce backwards into the run in front of it), and the kept prefix is then marked text-stale -- so every kept page is digest-checked on the next read, and a page that really did move fails its (ea, kind) sequence check and sets stale_structure, which makes Program.listing rebuild. Safe by construction rather than by argument about how far a reflow can reach.","gate_added_and_proven":".auto/check_edit.py drives undefine at 1/4/16 bytes at three positions (90%, 50%, 5% through the segment) and compares the kept model against a rebuild, narrow and wide. 0 problems on echo and ls_ttl. Proven to FAIL by deliberately making truncate_from too permissive: it reported 'kept model has 5952 rows, a rebuild has 5950'.","HONEST_LIMITATION":"total_ms 25814 -> 25783 is flat: nothing in the bench edits an item. I wrote a phase for it and reverted it because it HANGS -- driving undefine from inside the pilot produces no output and is killed at the timeout, while the identical sequence against Program directly is fine, and the same sequence with a print between prog.listing(ea) and ensure_ea is also fine. That points at the app's background threads (after bump_items deletes the model, a _prime/_grow worker still in flight can re-register a fresh one and hold its _load_lock), which is the same area as the unexplained streaming-responsiveness item already in .auto/ideas.md. Shipping a hanging bench phase would be worse than not having one.","what_i_did_not_do":"I did not weaken the phase until it passed, and I did not claim the win in total_ms. It is carried by a direct measurement and a permanent gate.","next_action_hint":"the concurrency question is now blocking two things (a bench phase for edits, and the streaming-responsiveness fix). Working out why @work(exclusive=True) leaves thread workers running -- and giving _prime/_grow a way to actually retire -- is the highest-value next step, and it is a correctness/responsiveness matter as much as a perf one."}} {"run":43,"commit":"b6646aa","metric":25121.9,"metrics":{"lg_boot_ms":694.8,"lg_decomp_ms":2370.8,"lg_graph_ms":917.1,"lg_hex_ms":456.1,"lg_index_ms":69.4,"lg_listing_cold_ms":442.7,"lg_listing_warm_ms":498.8,"lg_nav_ms":6548.8,"lg_palette_ms":4.8,"lg_rename_ms":711.4,"lg_render_ms":224.9,"lg_search_ms":3484.7,"lg_split_ms":2619.3,"pure_graph_ms":220.7,"sm_boot_ms":448.6,"sm_decomp_ms":1292.5,"sm_graph_ms":748.8,"sm_hex_ms":438.3,"sm_index_ms":2.4,"sm_listing_cold_ms":274.3,"sm_listing_warm_ms":266.5,"sm_nav_ms":302.7,"sm_palette_ms":0.3,"sm_rename_ms":392.5,"sm_render_ms":260.2,"sm_search_ms":59.6,"sm_split_ms":1370.9,"fails":0},"status":"keep","description":"The page-freshness check carries the digest the client already holds (heads(expect=...)) instead of asking first and fetching afterwards. A page that has NOT changed costs one round trip as before; a page that HAS changed now costs one instead of two. Also corrects the record: the item-edit bench hang is pilot start-up flakiness, not the _prime/_grow concurrency I blamed it on — proved with a stack dump.","timestamp":1786083427343,"segment":6,"confidence":4.621649308519722,"asi":{"gains":"total 25783 -> 25122 (-2.6%, best v7); lg_search 3886 -> 3485; sm_rename 416 -> 393. Direct measurements: post-rename whole-segment re-read 3720 -> 3411ms; getting the listing back after a rename on bash 21.5 -> 15.6ms (319x vs a rebuild); after an item edit 19 -> 17ms.","the_change":"heads gains `expect` (the digest a caller already holds) in place of the boolean `digest` flag. The worker builds the rows either way and omits them only when they still hash to `expect`. Sending the expectation rather than asking first is what removes the second round trip on a changed page -- which was the regression I introduced in v7 #10 (rename 380 -> 424ms) and flagged in my own next_action_hint.","equivalence":"normal calls (no expect) compared payload-for-payload against the previous commit across processes: 166 payloads, 63 964 rows, 0 mismatches. check_rename and check_edit both clean, 830-check gate green.","DIAGNOSIS_CORRECTED":"last iteration I logged that the item-edit bench phase hangs because 'after bump_items deletes the model, a _prime/_grow worker still in flight can re-register a fresh one and hold its _load_lock'. A stack dump (faulthandler.dump_traceback_later) says otherwise: at the moment of the hang there are NO idatui threads at all -- the main thread is idle in selectors.select() and everything else is an idle asyncio executor thread, and the app is stuck BEFORE app.run_test() returns. It is pilot start-up flakiness, nothing to do with bump_items or _load_lock. Ruled out: the kitty-graphics query (IDATUI_KITTY=0 still hangs). Partly environmental: orphaned idatui/worker.py processes accumulate from runs killed by `timeout`, and clearing them let the next run boot -- but it recurred, so that is not the whole story.","operational_note":"kill stray workers between probe runs (pkill -f idatui/worker.py). Several of this session's confusing measurements were taken with orphans competing for the box.","what_this_means_for_the_backlog":"the 'why does @work(exclusive=True) leave thread workers running' item is NOT what blocks the item-edit bench phase. The two are separate: the streaming-responsiveness question is still open on its own evidence (112 vs 237 _grew reports), but the bench phase is blocked on pilot start-up reliability instead.","state":"v7 baseline 33243 -> 25122 (-24.4%). lg_nav 6549 and lg_search 3485 are both at their per-row floors; what is left of the worker's cost is 56% generate_disasm_line."}} +{"run":44,"commit":"7e4b593","metric":24513.8,"metrics":{"lg_boot_ms":693.2,"lg_decomp_ms":2347.3,"lg_graph_ms":931.2,"lg_hex_ms":460.3,"lg_index_ms":69.1,"lg_listing_cold_ms":434.2,"lg_listing_warm_ms":462.4,"lg_nav_ms":6639.6,"lg_palette_ms":4.7,"lg_rename_ms":699.9,"lg_render_ms":219.7,"lg_search_ms":3441.1,"lg_split_ms":2218,"pure_graph_ms":216.5,"sm_boot_ms":436.9,"sm_decomp_ms":1264.9,"sm_graph_ms":758.2,"sm_hex_ms":437.9,"sm_index_ms":2.3,"sm_listing_cold_ms":256.8,"sm_listing_warm_ms":256,"sm_nav_ms":309.5,"sm_palette_ms":0.3,"sm_rename_ms":379.1,"sm_render_ms":249.5,"sm_search_ms":59.2,"sm_split_ms":1266,"fails":0},"status":"keep","description":"decomp_map: memoise obj_id -> ea for the whole function instead of only comparing against the previous column. dstr() was 79% of the tool (24us a call) and items interleave, so foo(a, b) flips call->arg->call and re-formatted an item already seen: 106594 calls for 15417 lines of bash. Also corrects run #30's claim that the duplicate ida_hexrays.decompile is what costs -- a warm decompile is 0.01ms.","timestamp":1786084349637,"segment":6,"confidence":4.950123344769902,"asi":{"hypothesis":"the split view is the 3rd largest term and run #30 left a named suspect behind; profile decomp_map's sweep instead of trusting the note","gains":"total 25122 -> 24514 (-2.4%, best v7). lg_split 2619 -> 2218, sm_split 1371 -> 1266. Direct: decomp_map over bash's 12 largest 4783 -> 2972ms, echo's 12 431 -> 345ms.","profile_that_drove_it":"/tmp/sweepprof.py over 15417 lines of bash: dstr() 2581ms (79%, 106594 calls @ 24.2us), get_line_item 691ms (445337 @ 1.55us), tag_remove 14ms. The existing dedupe only compared against the PREVIOUS column's obj_id, but ctree items interleave -- foo(a, b) alternates call/arg/call/arg -- so every flip re-formatted an item already seen. obj_id is unique within a cfunc, so a function-scoped memo is exact.","equivalence":"/tmp/mapdiff.py execs the decoded BODY of the current and the previous commit against the same cfunc and compares the whole map: echo 62 functions 0 mismatches, bash 250 functions 0 mismatches. NOTES counters unchanged (sm_split_mapped_lines 984, lg_split_mapped_lines 2070).","CORRECTS_THE_RECORD_1":"run #30 said what is left in decomp_map is the duplicate ida_hexrays.decompile. Wrong: /tmp/hxcache.py shows a warm decompile is 0.01ms -- Hex-Rays' own cache is free and the duplicate costs nothing. The 2970ms I first attributed to it was the decompile TOOL's own text/spans building.","CORRECTS_THE_RECORD_2":"I suspected the split view's text and its decomp_map came from different ctrees after a rename (/tmp/mapalign.py showed 6 of 8 differing). Not a bug: the probe used DECOMP_NO_CACHE, which the app never does. Program.decompile calls the force_recompile tool (which does exist) before refetching, so the plain decompile in decomp_map then hits the repopulated cache.","rejected_and_why":"cfunc.refresh_func_ctext() after a rename is 46x faster than the forced recompile (31ms vs 1432ms for ten functions) but only 2/10 reproduce the recompile's text -- the rest differ by Hex-Rays TYPE INFERENCE (char* vs const char*, unsigned int a4 vs int a4). That is a change to what is on screen, so it is a product decision, not a perf change. Recorded in .auto/ideas.md with the probe.","next_action_hint":"the remaining decomp_map cost is get_line_item per screen column (691ms/15k lines), and the sound way down is to map only the ~40 lines the pane can show -- the same lazy shape that won for search highlight ranges. It needs a windowed tool plus a lazy container because app.py and trace_ctl.py both index the whole list. Do NOT step over columns: a one-character variable would be skipped and its EA silently lost."}} +{"run":45,"commit":"pending","metric":25367.3,"metrics":{"lg_boot_ms":740.4,"lg_decomp_ms":2307.8,"lg_graph_ms":1227.6,"lg_hex_ms":450.2,"lg_index_ms":81,"lg_listing_cold_ms":425.7,"lg_listing_warm_ms":403.5,"lg_nav_ms":6778.2,"lg_palette_ms":4.9,"lg_rename_ms":734.7,"lg_render_ms":220.1,"lg_search_ms":3619.6,"lg_split_ms":2589.3,"pure_graph_ms":216.6,"sm_boot_ms":441.2,"sm_decomp_ms":1210,"sm_graph_ms":727,"sm_hex_ms":421.5,"sm_index_ms":2.4,"sm_listing_cold_ms":258.1,"sm_listing_warm_ms":257.3,"sm_nav_ms":300.8,"sm_palette_ms":0.3,"sm_rename_ms":377.4,"sm_render_ms":249.3,"sm_search_ms":58.9,"sm_split_ms":1263.5,"fails":0},"status":"discard","description":"Replace ida-pro-mcp's decompile_function_safe with a loop that allocates one ctree_item_t instead of three per line and memoises the per-line dstr() by obj_id. Directly measured through the real worker at 3.6x (2695 -> 746ms for the post-processing of bash's 8 largest), and byte-identical over 500 functions — but total_ms rose 24514 -> 25367 on phases this cannot touch (graph +297, nav +138, search +179, boot +47) with the box at load 2.70 vs 2.16. Re-running to separate the win from the drift.","timestamp":1786085063251,"segment":6,"confidence":4.932640144665465,"asi":{"hypothesis":"the decompile TOOL's post-processing is a large hidden cost: a warm Hex-Rays decompile is 0.01ms, yet re-running the tool on bash's 12 largest still cost 2951ms. The culprit is ida-pro-mcp's decompile_function_safe, which has the SAME three faults I fixed in decomp_map -- three ctree_item_t SWIG allocations per line (two never read) and a dstr() per line at 24us.","direct_measurement":"/tmp/decprof.py: 18991 lines of bash, 2302.6ms -> 429.0ms (5.37x), 121.2 -> 22.6 us/line, 0 mismatches. End to end through the real worker (/tmp/verifybind.py, the decompile TOOL with Hex-Rays warm, bash's 8 largest): 2695ms -> 746ms, i.e. 3.6x, so the rebinding definitely takes effect.","equivalence":".auto/check_decomp.py runs BOTH implementations against the same cfunc with include_addresses both ways: 128/128 echo and 372/372 ls_ttl byte-identical. Proven to be a real gate by keying the memo on it.op instead of it.obj_id: 69 problems, 59/128 passing.","why_discarded":"total_ms 24514 -> 25367. The phases this change touches improved (lg_decomp 2347 -> 2308, sm_decomp 1265 -> 1210), but graph +297, nav +138, search +179, boot +47 and index +12 all moved too, and NONE of them go through decompile_function_safe. Load average was 2.70 at the start of this run against 2.16 for the previous one. Per the playbook, totals are not comparable across a load change.","rollback_reason":"primary metric worse, but attributed to machine load rather than the change -- the same shape as #16/#18, both of which were confirmed wins on a re-run.","next_action_hint":"re-apply from /tmp/fastdecomp.patch (git diff saved before the auto-revert; .auto/check_decomp.py also copied to /tmp) and re-run. Load was 0.84 immediately after this run, so the box has calmed. NOTE: checks.sh now calls .auto/check_decomp.py, and .auto files survive the revert -- so the gate will CRASH until the patch is re-applied, because _idatui_decompile_function_safe will not exist in BODY."}} +{"run":46,"commit":"pending","metric":25171.3,"metrics":{"lg_boot_ms":693.5,"lg_decomp_ms":2317.5,"lg_graph_ms":879.1,"lg_hex_ms":449.8,"lg_index_ms":73.6,"lg_listing_cold_ms":437.1,"lg_listing_warm_ms":461.9,"lg_nav_ms":6738.4,"lg_palette_ms":4.8,"lg_rename_ms":702.8,"lg_render_ms":226.2,"lg_search_ms":3639,"lg_split_ms":2578,"pure_graph_ms":216.2,"sm_boot_ms":435.2,"sm_decomp_ms":1278.7,"sm_graph_ms":742.7,"sm_hex_ms":436.6,"sm_index_ms":2.5,"sm_listing_cold_ms":268.2,"sm_listing_warm_ms":263.5,"sm_nav_ms":289.5,"sm_palette_ms":0.3,"sm_rename_ms":383.3,"sm_render_ms":262.9,"sm_search_ms":58.1,"sm_split_ms":1332.1,"fails":0},"status":"discard","description":"Re-run of #45 (fast decompile_function_safe) with the rebinding no longer importing a module to patch it. Confirms the change does NOT move total_ms: three runs with it (25367/25058/25171) against three without (24514/25122/25783), means 25199 vs 25140. The 3.6x is real but lands on large functions (98us/line saved) while the bench decompiles 286-line ones (30us/line). Parked in .auto/parked/ rather than deleted.","timestamp":1786085672802,"segment":6,"confidence":5.315956151035322,"asi":{"hypothesis":"ida-pro-mcp's decompile_function_safe has the same three faults I fixed in decomp_map: three ctree_item_t SWIG allocations per line (two never read) and a dstr() per line at 24us","confirmed_true_but_small":"the optimisation itself is real and verified three ways -- /tmp/decprof.py 2302 -> 429ms over 18991 lines of bash (121.2 -> 22.6 us/line); through the REAL worker /tmp/verifybind.py 2695 -> 746ms; echo 219 -> 120ms. Byte-identical over 128/128 echo and 372/372 ls_ttl functions with include_addresses both ways.","why_it_does_not_show":"the saving is 98us/line on bash's LARGEST functions but only 30us/line on small ones, because dstr() cost and memo hit rate both scale with ctree size. The bench's fixed set averages 286 lines a function, so the expected effect is 150-250ms against a run-to-run spread of 400-600ms on this box. Three runs with (25367/25058/25171) vs three without (24514/25122/25783): means 25199 vs 25140.","rollback_reason":"primary metric unchanged within noise, and the best single run remains one without the change. Rules say discard on worse-or-unchanged, and unlike #42 the bench DOES cover this path -- it simply covers it with functions too small for the win to matter.","what_i_refused_to_do":"the obvious way to make this show up is to point the bench's fixture picker at larger functions. That is fitting the benchmark to the change, so I did not do it, and I recorded the prohibition next to the parked patch.","preserved":".auto/parked/fast_decompile.patch and .auto/parked/check_decomp.py, with the full measurement table in .auto/ideas.md. I also UNWIRED check_decomp.py from checks.sh before logging: .auto files survive the auto-revert but BODY does not, so the gate would have crashed on every subsequent run.","still_a_user_win":"an F5 on a 2374-line function loses 233ms of post-processing (287 -> 54). Worth re-applying if the goal moves from total session time to per-operation latency.","next_action_hint":"stop mining the decompile path -- what is left there is Hex-Rays. The largest terms are lg_nav 6700 and lg_search 3600, both at their per-row floors, so the next real find is likely another unbenched feature (xrefs dialog, strings browser, history, literal formats) probed with the /tmp/featprobe.py shape."}} diff --git a/.auto/parked/check_decomp.py b/.auto/parked/check_decomp.py new file mode 100644 index 0000000..0f30a03 --- /dev/null +++ b/.auto/parked/check_decomp.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Differential gate for the fast decompile_function_safe (run by checks.sh). + +`idatui/worker.py` rebinds ida-pro-mcp's `decompile_function_safe` to our own +loop, which skips two of the three SWIG allocations per line and memoises the +per-line `dstr()` by ctree obj_id. That is a pure speed change and the text it +returns is what the pseudocode pane shows, markers and all -- so it has to be +byte-identical, not merely similar. + +This runs BOTH implementations against the same cfunc for every function of a +real binary and compares the strings, with `include_addresses` both ways (the +marker path is the whole point, and the no-marker path must not regress either). + + ~/ida-venv/bin/python .auto/check_decomp.py [targets/echo] [max_funcs] +""" +from __future__ import annotations + +import os +import shutil +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.dirname(HERE) +sys.path.insert(0, ROOT) +sys.path.insert(0, HERE) + +from bench import stage # noqa: E402 + + +def _add_mcp_path() -> None: + """ida-pro-mcp is installed for the interpreter the WORKER runs, which is + not necessarily the one running this check.""" + import glob + for pat in ("/home/user/.local/lib/python3.*/site-packages", + os.path.expanduser("~/.local/lib/python3.*/site-packages")): + for d in glob.glob(pat): + if os.path.isdir(os.path.join(d, "ida_pro_mcp")) and d not in sys.path: + sys.path.append(d) + + +def main() -> int: + target = sys.argv[1] if len(sys.argv) > 1 else "targets/echo" + limit = int(sys.argv[2]) if len(sys.argv) > 2 else 400 + d, path = stage(os.path.join(ROOT, target)) + os.environ["IDA_MCP_TOOL_TIMEOUT_SEC"] = "0" + _add_mcp_path() + try: + import idapro + idapro.open_database(path, run_auto_analysis=True) + try: + import ida_funcs + import ida_hexrays + import idautils + from ida_pro_mcp.ida_mcp import utils + + original = utils.decompile_function_safe + sys.path.insert(0, os.path.join(ROOT, "server")) + import patch_server + # exec only our function out of the decoded BODY: the rest of it + # needs api_types' namespace (@tool, @idasync, ...). + body = patch_server.BODY + i = body.find("def _idatui_decompile_function_safe(") + j = body.find("\n_idatui_strings_cache", i) + assert i > 0 and j > i, "could not slice the function out of BODY" + g = {} + exec(body[i:j], g) + fast = g["_idatui_decompile_function_safe"] + + ida_hexrays.init_hexrays_plugin() + fails: list[str] = [] + n = ok = 0 + for ea in idautils.Functions(): + f = ida_funcs.get_func(ea) + if not f: + continue + n += 1 + if n > limit: + break + for markers in (True, False): + a, ea_err = original(f.start_ea, include_addresses=markers) + b, eb_err = fast(f.start_ea, include_addresses=markers) + if a != b or (ea_err is None) != (eb_err is None): + fails.append(f"{f.start_ea:#x} (markers={markers})") + if len(fails) <= 3: + la = (a or "").splitlines() + lb = (b or "").splitlines() + i = next((k for k, (x, y) in enumerate(zip(la, lb)) + if x != y), None) + if i is None: + print(f" FAIL {f.start_ea:#x}: {len(la)} lines " + f"vs {len(lb)}, errs {ea_err!r}/{eb_err!r}") + else: + print(f" FAIL {f.start_ea:#x} line {i}:") + print(f" original: {la[i][:100]!r}") + print(f" fast : {lb[i][:100]!r}") + break + else: + ok += 1 + print(f"decompile text: {ok}/{min(n, limit)} functions of {target} " + f"byte-identical to ida-pro-mcp's own loop, " + f"{len(fails)} problems") + return 1 if fails else 0 + finally: + idapro.close_database(save=False) + finally: + shutil.rmtree(d, ignore_errors=True) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.auto/parked/fast_decompile.patch b/.auto/parked/fast_decompile.patch new file mode 100644 index 0000000..c5a4ceb --- /dev/null +++ b/.auto/parked/fast_decompile.patch @@ -0,0 +1,122 @@ +diff --git a/idatui/worker.py b/idatui/worker.py +index 556e69a..04252cc 100644 +--- a/idatui/worker.py ++++ b/idatui/worker.py +@@ -119,6 +119,36 @@ def recv(sock: socket.socket): + # --------------------------------------------------------------------------- # + # worker + # --------------------------------------------------------------------------- # ++def _use_fast_decompile() -> None: ++ """Point ida-pro-mcp's decompile tools at our per-line loop. ++ ++ The shipped ``decompile_function_safe`` allocates three ctree_item_t SWIG ++ objects per pseudocode line (two of which it never reads) and formats an ++ item description per line to recover the ``/*0xEA*/`` marker: 121us a line, ++ which on a warm cfunc is most of what the tool costs. The replacement lives ++ in server/patch_server.py and is differentially checked against the original ++ by .auto/check_decomp.py. ++ ++ Every consumer binds the name at import time (``from .utils import ...``), ++ so rebinding it on ``utils`` alone would miss them; rebind on each module ++ that imported it, and leave anything unexpected exactly as it was. ++ """ ++ try: ++ from ida_pro_mcp.ida_mcp import api_types, utils ++ fast = api_types._idatui_decompile_function_safe ++ except Exception: # noqa: BLE001 -- never let this stop the worker booting ++ return ++ utils.decompile_function_safe = fast ++ # Rebind only on modules that are ALREADY imported. Importing one to rebind ++ # it would be work the worker had not chosen to do, on the boot path. ++ prefix = "ida_pro_mcp.ida_mcp." ++ for name, mod in list(sys.modules.items()): ++ if not name.startswith(prefix) or mod is None: ++ continue ++ if getattr(mod, "decompile_function_safe", None) is not None: ++ mod.decompile_function_safe = fast ++ ++ + def _ensure_tools_injected() -> None: + """Inject idatui's custom tools (heads/read_raw/resolve_names/func_types/...) + into the installed ida_pro_mcp, idempotently, so the worker is self-sufficient +@@ -190,6 +220,8 @@ def _open_and_register(binpath: str, load_args: str = ""): + # importing the package registers all api_*/patched tools against MCP_SERVER + from ida_pro_mcp.ida_mcp import MCP_SERVER # noqa: WPS433 + ++ _use_fast_decompile() ++ + import ida_nalt + module = os.path.basename(ida_nalt.get_root_filename() or binpath) + +diff --git a/server/patch_server.py b/server/patch_server.py +index 6667e12..64424e4 100644 +--- a/server/patch_server.py ++++ b/server/patch_server.py +@@ -1039,6 +1039,67 @@ def decomp_map( + return {"addr": hex(func.start_ea), "lines": lines} + + ++def _idatui_decompile_function_safe(ea, include_addresses=True): ++ """ida-pro-mcp's ``decompile_function_safe``, with the three costs the same ++ sweep had in ``decomp_map`` taken out. Byte-identical output -- it is ++ differentially checked against the original over every function of a real ++ binary by ``.auto/check_decomp.py``. ++ ++ The shipped version costs 121us per pseudocode line, which is more than the ++ line's share of Hex-Rays itself on a warm cfunc: ++ ++ * it allocates THREE ctree_item_t SWIG objects per line, and ``_head`` and ++ ``_tail`` are never read -- ``get_line_item`` accepts None for both. ++ * it calls ``dstr()`` per line. That formats a whole 'EA: description' ++ string at 24us a call, and consecutive lines of a multi-line expression ++ report the same ctree item, so memoising by ``obj_id`` (unique within a ++ cfunc) skips most of them. ++ ++ 18 991 lines of bash: 2 302ms -> 429ms. ++ """ ++ import ida_lines ++ import ida_hexrays as _hx ++ from ida_pro_mcp.ida_mcp.utils import compact_whitespace, decompile_checked ++ from ida_pro_mcp.ida_mcp.sync import IDAError ++ try: ++ cfunc = decompile_checked(ea) ++ item = _hx.ctree_item_t() ++ get_line_item = cfunc.get_line_item ++ tag_remove = ida_lines.tag_remove ++ ea_of_id = {} ++ lines = [] ++ for sl in cfunc.get_pseudocode(): ++ line = sl.line ++ line_ea = None ++ if include_addresses and get_line_item(line, 0, False, None, ++ item, None): ++ it = item.it ++ oid = it.obj_id if it is not None else None ++ if oid is not None and oid in ea_of_id: ++ line_ea = ea_of_id[oid] ++ else: ++ dstr = item.dstr() ++ if dstr: ++ ds = dstr.split(": ") ++ if len(ds) == 2: ++ try: ++ line_ea = int(ds[0], 16) ++ except ValueError: ++ pass ++ if oid is not None: ++ ea_of_id[oid] = line_ea ++ text = compact_whitespace(tag_remove(line)) ++ if line_ea is not None: ++ lines.append(f"{text} /*{line_ea:#x}*/") ++ else: ++ lines.append(text) ++ return "\\n".join(lines), None ++ except IDAError as e: ++ return None, str(e) ++ except Exception as e: ++ return None, f"Decompilation failed at {hex(ea)}: {e}" ++ ++ + _idatui_strings_cache = {} + + diff --git a/.auto/parked/fast_pc_nums.patch b/.auto/parked/fast_pc_nums.patch new file mode 100644 index 0000000..f06bfe1 --- /dev/null +++ b/.auto/parked/fast_pc_nums.patch @@ -0,0 +1,52 @@ +diff --git a/server/patch_server.py b/server/patch_server.py +index 6667e12..5e20671 100644 +--- a/server/patch_server.py ++++ b/server/patch_server.py +@@ -1900,7 +1900,7 @@ def _idatui_lit_extent(plain, x): + return (lo, hi) + + +-def _idatui_pc_nums(cf, sl): ++def _idatui_pc_nums(cf, sl, plain=None): + """Every number literal on one pseudocode line, as + [{x0, x1, ea, opnum, value, nbytes, fmt}]. + +@@ -1912,16 +1912,26 @@ def _idatui_pc_nums(cf, sl): + import ida_lines + import idaapi + +- plain = ida_lines.tag_remove(sl.line) ++ # ``plain`` is the untagged line; callers that already have it pass it in ++ # rather than making tag_remove run twice over every line of the function. ++ if plain is None: ++ plain = ida_lines.tag_remove(sl.line) + out = [] + x = 0 ++ # One ctree_item_t for the whole line, and no head/tail at all. They are ++ # SWIG allocations in the innermost loop of a scan that probes every ++ # literal-looking character -- and 'a' to 'f' are hex digits, so `a1`, `v6` ++ # and `sub_1F4C0` all qualify and most columns of a line get probed. head ++ # and tail were never read. (Same three costs as decomp_map's sweep.) ++ item = ida_hexrays.ctree_item_t() ++ line = sl.line ++ get_line_item = cf.get_line_item + while x < len(plain): + ch = plain[x] + if ch not in _IDATUI_LIT_CHARS and ch != "'": + x += 1 + continue +- head, item, tail = (ida_hexrays.ctree_item_t() for _ in range(3)) +- if not cf.get_line_item(sl.line, x, True, head, item, tail): ++ if not get_line_item(line, x, True, None, item, None): + x += 1 + continue + if item.citype != ida_hexrays.VDI_EXPR: +@@ -1997,7 +2007,7 @@ def pc_nums( + for i in range(len(sv)): + plain = ida_lines.tag_remove(sv[i].line) + compact = _idatui_compact(plain) +- for rec in _idatui_pc_nums(cf, sv[i]): ++ for rec in _idatui_pc_nums(cf, sv[i], plain): + out.append({ + "line": i, + "x0": _idatui_compact_col(plain, compact, rec["x0"]), |