Skip to content

Commit 8710af0

Browse files
authored
[ty] lower MAX_RECURSIVE_UNION_LITERALS (#23521)
## 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
1 parent 784c518 commit 8710af0

2 files changed

Lines changed: 4 additions & 4 deletions

File tree

crates/ty_python_semantic/resources/mdtest/call/union.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,16 +299,16 @@ class RecursiveAttr2:
299299
self.i = 0
300300

301301
def update(self):
302-
self.i = (self.i + 1) % 9
302+
self.i = (self.i + 1) % 4
303303

304-
reveal_type(RecursiveAttr2().i) # revealed: Unknown | Literal[0, 1, 2, 3, 4, 5, 6, 7, 8]
304+
reveal_type(RecursiveAttr2().i) # revealed: Unknown | Literal[0, 1, 2, 3]
305305

306306
class RecursiveAttr3:
307307
def __init__(self):
308308
self.i = 0
309309

310310
def update(self):
311-
self.i = (self.i + 1) % 10
311+
self.i = (self.i + 1) % 5
312312

313313
# Going beyond the MAX_RECURSIVE_UNION_LITERALS limit:
314314
reveal_type(RecursiveAttr3().i) # revealed: Unknown | int

crates/ty_python_semantic/src/types/builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ impl RecursivelyDefined {
256256

257257
/// If the value ​​is defined recursively, widening is performed from fewer literal elements,
258258
/// resulting in faster convergence of the fixed-point iteration.
259-
const MAX_RECURSIVE_UNION_LITERALS: usize = 10;
259+
const MAX_RECURSIVE_UNION_LITERALS: usize = 5;
260260
/// If the value ​​is defined non-recursively, the fixed-point iteration will converge in one go,
261261
/// so in principle we can have as many literal elements as we want,
262262
/// but to avoid unintended huge computational loads, we limit it to 256.

0 commit comments

Comments
 (0)