Skip to content

Conversation

@sterliakov
Copy link
Collaborator

Fixes #20107.

One recursive alias is enough to trigger infinite recursion - why does this require that both sides are aliases?

Cc @ilevkivskyi as original author of #16483.

@github-actions

This comment has been minimized.

@ilevkivskyi ilevkivskyi marked this pull request as ready for review October 27, 2025 16:33
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it was a performance thing. I don't remember doing any measurements, but it is some hot code. Probably the most correct way would be to use mypy.typeops.is_recursive_pair() (but to be clear that function itself is a hack as well, but it is good to have it centralized). Could you please do some performance measurements for this PR? (and also for is_recursive_pair if you have time), otherwise LG.

@sterliakov
Copy link
Collaborator Author

I wanted to run some perf checks on this, but don't remember any typealias-heavy codebase to use. Selfcheck almost definitely won't show anything - there are barely any type aliases in mypy code - and isolated benchmarks won't be representative in this case. Any suggestions?

@ilevkivskyi
Copy link
Member

@sterliakov IIRC altair from mypy primer is quite sensitive to type aliases.

@sterliakov
Copy link
Collaborator Author

Timed for _ in {1..8}; do time mypy altair tests >/dev/null; rm -rf .mypy_cache/; done using mypyc-compiled version at -O3 (gcc 13.3.0 on linux) and the same dependencies as installed by primer. First two warm-up runs were discarded. Results are given as $\text{mean} \pm \text{sd}$ in seconds (computed with stdlib statistics.fmean and statistics.stdev, resp.).

  • PR: $31.2 \pm 0.4$
  • is_recursive_pair: $30.9 \pm 0.3$
  • master: $31.3 \pm 0.4$

There are 210 matches of \bTypeAlias\b in altair, and they do indeed seem to use them heavily, so this was a good benchmark, thanks!

So I'm switching to is_recursive_pair because it is not significantly slower (and even appears to be faster which is reasonable), and there's no significant perf hit from this PR compared to master - all numbers are equal within 1 SD.

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@ilevkivskyi ilevkivskyi merged commit 841db1f into python:master Oct 28, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault crash on specific recursive type situation

2 participants