Lazily create builtin bindings#26510
Conversation
Merging this PR will improve performance by 9.79%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | linter/default-rules[numpy/globals.py] |
179.1 µs | 148.7 µs | +20.42% |
| ⚡ | Memory | linter/default-rules[pydantic/types.py] |
348.9 KB | 306.7 KB | +13.78% |
| ⚡ | Memory | linter/default-rules[numpy/ctypeslib.py] |
164.2 KB | 146.2 KB | +12.31% |
| ⚡ | Simulation | linter/all-with-preview-rules[numpy/globals.py] |
832.5 µs | 749.1 µs | +11.13% |
| ⚡ | Simulation | linter/all-rules[numpy/globals.py] |
717 µs | 664.6 µs | +7.88% |
| ⚡ | Memory | linter/all-rules[pydantic/types.py] |
648.8 KB | 606.6 KB | +6.97% |
| ⚡ | Simulation | linter/default-rules[unicode/pypinyin.py] |
370.1 µs | 346.3 µs | +6.87% |
| ⚡ | Memory | linter/all-with-preview-rules[pydantic/types.py] |
667.2 KB | 625 KB | +6.76% |
| ⚡ | Memory | linter/all-rules[numpy/ctypeslib.py] |
289.3 KB | 271.3 KB | +6.64% |
| ⚡ | Memory | linter/all-with-preview-rules[numpy/ctypeslib.py] |
317.3 KB | 299.3 KB | +6.01% |
Tip
Curious why this is faster? Use the CodSpeed MCP and ask your agent.
Comparing charlie/codex-codspeed-speedup (634be4d) with main (71d151f)
Footnotes
-
93 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
675a38b to
8e9a3a6
Compare
|
Nice! The performance improvement looks great, but I'm a bit wary of the API. It seems like it would be easy to forget to call As one example, Codex claims that this snippet: from airflow.operators.python import PythonOperator
task = PythonOperator(
task_id="task",
op_args="{{ ti.xcom_pull(task_ids='len') }}",
)ruff check --preview --select AIR201 --unsafe-fixes repro.pyhas no fix on this branch but offers a fix on main. The result of the fix actually looks like a bug on main (playground), but it's still an example of a behavior change that I think is accidental. In general Codex claims "every caller must remember whether |
|
|
Good feedback -- how does this look? |
ntBre
left a comment
There was a problem hiding this comment.
Looks great to me, thank you!
Summary
The linter previously inserted every version-specific builtin, magic global, notebook builtin, and configured custom builtin into the module scope before walking the AST, even when a file never referenced most of those names.
This moves builtin identification into
SemanticModeland creates concrete builtin bindings only when a load or binding operation needs them.