[ty] lower MAX_RECURSIVE_UNION_LITERALS#23521
Merged
carljm merged 1 commit intoastral-sh:mainfrom Feb 24, 2026
Merged
Conversation
Typing conformance resultsNo changes detected ✅ |
|
Memory usage reportSummary
Significant changesClick to expand detailed breakdowntrio
sphinx
prefect
|
Merging this PR will improve performance by 4.3%
Performance Changes
Comparing Footnotes
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-argument-type |
0 | 1 | 0 |
invalid-return-type |
1 | 0 | 0 |
type-assertion-failure |
0 | 1 | 0 |
| Total | 41 | 2 | 0 |
AlexWaygood
approved these changes
Feb 24, 2026
Member
AlexWaygood
left a comment
There was a problem hiding this comment.
This seems very reasonable to me, given there is no ecosystem fallout and a significant performance improvement. I'd prefer to get signoff from at least one other maintainer here, though!
sharkdp
approved these changes
Feb 24, 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
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
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
LoopHeaderbranch ofinfer_loop_header_definitionandplace_from_bindings_implwill 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_LITERALSfrom 10 to 5. This will result in faster convergence when the loop variable grows likeLiteral[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:
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