Repository navigation
Use a flat Python AST in Ruff and ty - #29817
MichaReiser wants to merge 47 commits into
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 98.60%. The percentage of expected errors that received a diagnostic held steady at 98.86%. The number of fully passing files held steady at 137/146. Results for 0936621 · Workflow run |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
trio
sphinx
prefect
Results for 0936621 · Workflow run |
|
|
| let rewritten = owned_pattern_to_expr(pattern.view(storage).to_owned_node()); | ||
| let id = ast::flat::Lower::lower(&rewritten, storage); |
There was a problem hiding this comment.
[P3] Avoid retaining entire subtrees during pattern recovery
A malformed class pattern whose head is a sequence reaches this conversion at each nesting level. to_owned_node() copies the entire previously recovered subtree, and Lower::lower appends its literal bytes to the same Builder without reclaiming the originals. Nesting around one large string therefore retains memory proportional to literal size × nesting depth, allowing an attacker-controlled file in an untrusted language-server workspace to exhaust host memory. The previous implementation moved owned children. Construct the recovery nodes using existing child handles to avoid retaining these repeated copies.
Merging this PR will regress 5 benchmarks1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
bd231d4 to
59e0aa9
Compare
|
YES |
46a2f59 to
8b89865
Compare
8b89865 to
1d9bbf2
Compare
Reuse the expression or statement kind across checker dispatch and child traversal. Visitors that specialize a node can forward the kind they already matched. Pattern and type-parameter walkers use the same convention, and the exception-handler walker accepts its concrete node. Keep the capture name across pattern traversal so mapping patterns look up their rest capture once while preserving binding order.
Summary
Ruff and ty parse Python into 24-byte rows linked by four-byte node IDs. Typed nodes borrow fields from the parsed module, lint fixes build replacement syntax in an arena, and ty stores row IDs without a separate node-numbering pass. Python AST storage and traversal
Review order
The PR starts at the boxed mapping-pattern layout in #29889. The first five commits migrate the AST and its consumers. The remaining 41 isolate subsequent changes. Both Ruff and ty build at the fifth commit. Boxed parent
0f8d2da79c.466cd822cca19708bfa516cfcafc3a1863efdc0437db863a96Performance
The boxed and flat revisions both contain #29916, which resolves a call’s qualified name once for the applicable lint rules. Both use source offsets for Ruff’s name-binding map. These measurements predate the comparison optimizations at the base of the stack. The measured flat executable used Git tree
2f1051bec9. Boxed48b74912c7· Flatd28e863169.Ruff lint
The five repository fixtures run with the default rules, all stable rules and all rules including preview. The benchmark parses each module before the timed lint call. Each row sums the same 15 jobs. Expression checking runs inside
check_ast, so these scopes overlap.check_astcheck_astBoth checkers dispatched 26,613 expressions. Among rules with matching native call counts on every fixture, 175 call rules account for 2,120,584 additional instructions and 17 comparison rules for 902,616. The shared callee-name resolution itself ran 2,925 times in each build and cost 22,413 additional instructions. Inlining prevents a separate count for every rule.
For
client.send(), S201 checks whether the method name isrun, and S610 checks whether it isextra. Each rule receives the decoded call fields. Each still reads the attribute’s identifier. The boxed attribute embeds the identifier. The flat attribute holds a four-byte ID, so the getter loads the ID and resolves the identifier row. S201 · S610Lint instructions by fixture and rule selection, excluding parsed-module destruction
numpy/globals.py· Defaultnumpy/globals.py· All stablenumpy/globals.py· All with previewnumpy/ctypeslib.py· Defaultnumpy/ctypeslib.py· All stablenumpy/ctypeslib.py· All with previewunicode/pypinyin.py· Defaultunicode/pypinyin.py· All stableunicode/pypinyin.py· All with previewpydantic/types.py· Defaultpydantic/types.py· All stablepydantic/types.py· All with previewlarge/dataset.py· Defaultlarge/dataset.py· All stablelarge/dataset.py· All with previewThe first complete flat migration used 161,812,123 instructions before destruction (+9.51% versus boxed). The subsequent commits reduced this to 152,327,272 (−5.86% versus the first flat version).
85f8507b0eLint instructions after each of the final five changes
117e774f6fb2ba50093f7436ae13a84e0503771cc72ee7afd8d28e863169Compare the two boxed layout PRs and the flat AST on the same 15 lint jobs
d71685096afd8861c83948b74912c785f8507b0ed28e863169Complete Ruff and ty commands
These profiling builds run the actual CLI commands with one worker. The before-destruction columns subtract the instructions in the parsed-module destructor from the same process. Both variants produced identical Ruff diagnostics, edits, formatter output and ty diagnostics.
The lint projects contain 568 CPython files, 25 Jinja2 files and 321 Pygments files. The formatter row uses the median whole-command result from three runs per layout on 568 files. Each ty project uses one matched pair.
Whole-command instructions and parsed-module destruction
Complete Ruff lint commands compared directly with boxed main
Plain boxed main, the boxed parent with both layout changes and the flat head use the same inputs. All three produced byte-identical CLI output.
Hosted CodSpeed
CodSpeed compared flat
d28e863169with its boxed parent48b74912c7. Its three reported regressions are CPU simulations of lintingnumpy/globals.py. Across all instruments it reported 167 improvements, 118 unchanged benchmarks and 14 skipped. The report flags different runner hardware for 127 benchmarks with significant changes. Hosted resultsThe local columns repeat the corresponding repository lint benchmark on the same fixture, including destruction of the parsed module. CodSpeed's CPU simulation models both executed instructions and cache-miss penalties. CPU simulation
In the preview benchmark,
check_astaccounts for 3,328 of the 3,515 additional L1 instruction-cache misses. L1 data-read misses fall from 2,636 to 2,408.How instruction-cache capacity affects the same file in the Ruff CLI
These runs collect instructions inside
check_pathin an isolated, single-file CLI invocation. Each pair produced identical diagnostics. The cache simulations use the same profiling binaries, eight-way instruction caches with 64-byte lines, and fixed data and last-level caches.Increasing the cache from 32 KiB to 4 MiB removed 74–76% of the extra misses. Holding capacity at 32 KiB and making the cache fully associative left gaps of 1,903, 3,892 and 3,408 misses, respectively. The flat CLI executes a larger set of machine instructions, and the smaller cache repeatedly fetches some of that code. These measurements establish the effect of capacity in the simulator. Quantifying how much inlining contributes would require a separate comparison.
Review the 41 commits after the basic migration
4671a24d28b28d6670e29197d82ec15df7b121700744f8273ad727ad2b497ae650097903d630d0268206b084bfb07cd8977a18af1be6eaf7c968677e24b2e68768d3ca11300a0c6de85d0deb56d0de96a604949e3069ddb091c763080828136d969ef5ba7c32da470db881497b3d5a1a58efaf0d1aecd042ebe7c2f7041ac46df1ccffb5aee12930a2b7ce292e9277139d7f761cdc1cc1dc702ec32dc5c501848b85abcec686268ac88077b925149c0047cd0beb8e9db15765c423b19c2797b87a93521717bf5c831890f9a1bdTest coverage
Parser fixtures and the existing lint and formatter suites exercise the arena through its consumers. Traversal tests cover source order for valid and recovered syntax. ty’s semantic tests cover resolving stored node IDs, including in quoted annotations.
A boxed-versus-flat check over 1,731 fixture and rule-setting pairs matched all 66,678 diagnostics and 29,731 fixes. A separate check exercised all 70 rules in the shared callee dispatch using direct, aliased, unresolved and shadowed callees and per-file ignores. All 228 diagnostics and 138 fixes matched, including applicability and edit text.