[ty] Track narrowing unions to prevent exponential blowup#22066
Conversation
When inferring a call expression with a union type context, we try narrowing to each element of the union. If nested calls have the same union as their parameter type, this would lead to exponential blowup. By tracking which unions we're already narrowing against, we skip redundant nested narrowing. On a synthetic benchmark with nested `list_schema()` calls: - 4-level nesting: >120s → 0.13s
zanieb
added
performance
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
CodSpeed Performance ReportMerging #22066 will improve performance by 72.68%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
Hm is this a fix for some false positives? |
c6058e4 to
3cf876e
Compare
dcreager
left a comment
dcreager
left a comment
There was a problem hiding this comment.
This looks fine to me, esp with an eye towards landing a quick fix and iterating on that as follow-on work.
@ibraheemdev mentioned in discord:
I think we need something slightly different than [this] PR though, we should be propagating the narrowed type context through the nested calls instead of the entire union, but not ignoring it completely
It looks like with the latest commit, you are propagating the narrowed type (though not the narrowed type context) along with the union. I'm not sure whether that's sufficient, but if not, I'm happy to address that as a follow-on.
|
Thanks for giving this a look!
Ah interesting. I missed the nuance there. |
It might be the case that the narrowed type context will always end up being the narrowed type — that's the point of the bidi type context after all. But I'm not certain whether there are cases where the narrowed type will still be narrower than the narrowed type context, and if so, whether that's important. I would defer to @ibraheemdev on that question. |
|
As discussed on Discord, I don't think this is correct. We already correctly propagate the narrowed type context during inference, this PR essentially argues that if a child call has a union as type context which we are currently narrowing as part of a parent call, the child call will narrow to the same element as the parent. It's very rare to have nested generic calls where this matters in the first place, but that heuristic is not necessarily true, e.g., def f(x: list[int | None] | list[str | None]) -> str:
return ""
def g[T](x: T) -> list[T]:
return [x]
# regression: error: Expected `list[int | None] | list[str | None]`, found `list[str | None | int]`
f(g(f(g(1))))The performance improvement here seems to be in cases where didn't need to narrow in the first place, which #22102 should address. |
|
Thank you! |
3 participants
See astral-sh/ty#2026 (comment)
I'm mostly exploring this as a short-term way to resolve the exponential complexity of recursive analysis as described in the linked issue. It may or may not have longstanding value once we support discriminated (or "tagged") unions for typed dicts.