Assign debug operation IDs in one pass per graph, not once per node - #87
Merged
gokulkrishna98 merged 1 commit intoSep 10, 2026
Merged
Conversation
`_DebugInfoRecorder._get_operations_for_node` ended with
`_in_ir_order(added_operations, self._current_graph)`, which built a position map by
walking every operation converted so far in order to sort the handful the current FX
node had just added. That runs once per node, so conversion is O(nodes x ops) --
quadratic in graph size.
The constant factor is large as well: each `for op in block` ends in a pybind11
stop_iteration, i.e. a C++ throw, and on macOS every throw calls
`_dyld_find_unwind_sections`, which re-parses the Mach-O headers of loaded images. A
`sample` of a real export shows `_Unwind_RaiseException`, `__gxx_personality_v0` and
`mach_o::Header::parse_segment_command` on top.
Operation IDs and debug locations are now assigned in a single IR-order pass once the
graph body is complete (`finalize_node_operations`), called from `_get_graph_op` after
the node loop. Measured on a chain of Linear+relu, `to_coreai()` alone:
layers before after
128 1.30s 0.07s
256 4.91s 0.14s
512 19.98s 0.28s (~70x)
i.e. quadratic becomes linear. A ten-procedure production model went from >45 min of
conversion, unfinished, to 48 seconds.
Two details worth review attention:
* `update_output_maps` now guards on the target having debug info rather than on it
having an ID. IDs are assigned later, so the old guard would have silently dropped
every output map.
* Operation IDs change. They are now genuinely IR-ordered; previously they followed
lowering order, which is not the same thing, because a constant is inserted at the
head of the block rather than appended -- a chain of linears numbered
`[0, 3, 7, 10, ..., 1, 2, 4, 5, ...]` when read in IR order. Nothing pinned the old
values.
Adds three tests: IDs increase in IR order with none missing or repeated; output maps
survive; and the graph is walked a fixed number of times per graph rather than once per
lowered node. The last one is a counter rather than a timing assertion, so it is stable
in CI. Both of the first two fail on the parent commit.
cymbalrush
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
n^2issue for the debug info.