[ty] Isolate loop header reachability evaluation in tracked function#23520
Merged
mtshiba merged 4 commits intoastral-sh:mainfrom Feb 24, 2026
Merged
[ty] Isolate loop header reachability evaluation in tracked function#23520mtshiba merged 4 commits intoastral-sh:mainfrom
mtshiba merged 4 commits intoastral-sh:mainfrom
Conversation
Typing conformance resultsNo changes detected ✅ |
|
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
0 | 40 | 0 |
unresolved-attribute |
0 | 0 | 10 |
unsupported-operator |
0 | 0 | 3 |
invalid-argument-type |
1 | 0 | 0 |
invalid-return-type |
0 | 1 | 0 |
| Total | 1 | 41 | 13 |
mtshiba
requested review from
AlexWaygood,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
1 task
oconnor663
approved these changes
Feb 24, 2026
carljm
pushed a commit
that referenced
this pull request
Feb 24, 2026
## Summary This is a simpler approach to the performance issues mentioned in #23520. isort's profiling results suggested that #22794 added a slow-converging calculation rather than adding a specific hotspot to the type inferer. What makes type inference for loop variables more troublesome than other types of type inference is the calculation of reachability. For other types, such as implicit attribute type inference, reachability analysis is not performed for each attribute binding (reverted due to performance issues: #20128, astral-sh/ty#2117). They are all treated as reachable. Loop variables perform this heavy calculation (omitting reachability analysis from the `LoopHeader` branch of `infer_loop_header_definition` and `place_from_bindings_impl` will significantly improve performance). It appears that slow convergence for one variable in a loop block will also slow down the inference of all other definitions in the block that depend on it. To alleviate the issue, this PR reduces the value of `MAX_RECURSIVE_UNION_LITERALS` from 10 to 5. This will result in faster convergence when the loop variable grows like `Literal[0, 1, ...]`. Local measurements show that this PR alone improved the isort inspection time by about 37%. I chose 5 as the new value because I felt it offered a good balance between type inference precision and performance, based on the following measurement results: | Threshold | Mean | vs 10 | |-----------|--------|---------| | 4 | 0.89s | -38% | | 5 | 0.91s | -37% | | 6 | 1.05s | -27% | | 7 | 1.06s | -27% | | 8 | 1.23s | -14% | | 9 | 1.26s | -13% | | 10 (current) | 1.43s | — | It has been observed that this PR and another mitigation, #23520, are compatible, resulting in a total performance recovery of about 40-50%. ## Test Plan N/A
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
2 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.
Summary
Probably related: astral-sh/ty#2808
I noticed that #22794 made isort run about 30x slower than it was before.
https://548dfc80.ty-ecosystem-ext.pages.dev/timing
isort has very large loop blocks in its codebase (e.g. https://github.com/PyCQA/isort/blob/b5f06a7b1d53b7b561f00f40f814ef1698864bac/isort/core.py#L32).
We need to implement performance optimization solutions for these kinds of large loops.
In this PR, I implement one of the slowdown mitigation measures I came up with. The actual inference of the loop variable is done in two places:
infer_loop_header_definitionandplace_from_bindings_impl(theLoopHeaderbranch). Reachability analysis is performed independently here, but this calculation is very expensive and should be cached. Therefore, I add a tracked functionloop_header_reachability.This change has resulted in a 12-19% performance improvement for isort locally.
See also: #23521
cc: @oconnor663
Test Plan
N/A