[Lang] Memoise the per-node source-position banner on the function - #858
Merged
Conversation
ASTTransformerBase.__call__ attaches source-position info to every generated IR node via get_pos_info, which TextWrapper-formats a source-line-plus-caret hint. That ran for every stmt/expr node of every kernel compilation and dominated the Python side of a kernel build. A function is transformed once per inlined call site, so the same positions are formatted tens of times over. Memoise the result on the FuncBase, which is what fixes every input beyond the node itself, leaving the produced string byte-identical. On a kernel-build-heavy workload (~130 offloaded tasks), the first step after editing one kernel goes from ~10.6s to ~6.1s. get_pos_info is called 277k times there and the memo serves 97.4% of them.
This was referenced Aug 11, 2026
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Collaborator
Author
|
running genesis unit tests and bench |
Collaborator
Author
Collaborator
Author
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
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.


What
ASTTransformerBase.__call__attaches source-position info to every generated IR node viaget_pos_info, whichTextWrapper-formats a source-line-plus-caret hint. That formatting therefore runs for every stmt/expr node ofevery kernel compilation, and it dominates the Python side of a kernel build.
A function is transformed once per inlined call site -- tens of times over on a large kernel -- and each transform
re-formats the identical positions. This memoises the result. The string produced is byte-identical, so every
diagnostic that embeds it is unchanged.
The cache lives on the
FuncBaserather than in a module-level dict, because theFuncBaseis exactly what fixesthe remaining inputs:
file,src,indent,lineno_offsetand the function name all derive from it and areidentical across its transforms, so the key is just the node's position and type. That also bounds the cache by the
function's lifetime and keeps it correct across a module reload, which hands out new
FuncBaseobjects.Impact
On a kernel-build-heavy workload (a qipc solver step, ~130 offloaded tasks), timing the first step after editing
one kernel, interleaved over a single unchanged build:
get_pos_infois called 277k times in that run and the memo serves 97.4% of them.Relationship to #804
#804 attacks the same hotspot by attaching only a cheap
File "...", line Nheader per node and deferring the fullhint to the compile-error path. That is a smaller change and I tried it first (#857), but it is not
behaviour-preserving: the per-node info is not read only by compile errors, it is embedded in
DebugInfoandsurfaces in runtime diagnostics.
test_overflow.pyasserts the offending source line appears in the overflowmessage and 64 of its cases fail without it:
Memoising avoids that by construction, since the string never changes.
test_overflow.pyis green here.#804's other change, replacing the
O(len(sys.modules))scan in_inside_classwith alinecachelookup, isindependent of this and still worth landing on its own.
Testing
test_overflow.py,test_exception.py,test_syntax_errors.py,test_nested_kernel_error.pyandtest_ast_refactor.py-- the suites that assert on message text -- give 291 passed both with and without thechange, on the same build.
Made with Cursor