Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance in union subtyping #15104

Merged
merged 1 commit into from
Apr 23, 2023
Merged

Conversation

hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Apr 23, 2023

This is a performance optimisation for subtyping between two unions that are largely the same.

Fixes #14034

This makes @adriangb's example in #14034 (comment) finish basically instantly. I could add it as a unit test?

Type checking pydantic core is still not fast — takes like four or five minutes with uncompiled mypy — but at least it's now feasible. I think there's room for doing some optimisation in make_simplified_union that would improve this.

This is a performance optimisation for subtyping between two unions that
are largely the same.

Fixes python#14034
@adriangb
Copy link
Contributor

This is amazing! It's a huge (>99%) improvement.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@dmontagu
Copy link

dmontagu commented Apr 23, 2023

Massive improvement, I compiled this locally and was able to type check the main branches of pydantic and pydantic-core (with our if not MYPY: hack removed in pydantic_core.core_schema) in ~26s on my machine (using python 3.11). And that was without the --disable-recursive-aliases flag that we used to need for it to finish at all; no one ever waited long enough to see how long it would take without it, but it was at least hours. (I'll note for anyone curious that with these changes, that flag no longer has any effect on performance while type-checking pydantic.)

For comparison, mypy 1.2.0 with --disable-recursive-aliases takes ~2 minutes on my machine.

This is fast enough that it is viable once again for us to use mypy again for type-checking pydantic, but we'd definitely appreciate any further performance improvements that might be possible from make_simplified_union!

@cdce8p cdce8p mentioned this pull request Apr 23, 2023
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Awesome!

@JukkaL JukkaL merged commit bd6ce23 into python:master Apr 23, 2023
@samuelcolvin
Copy link
Contributor

Awesome, thank you so much.

@hauntsaninja hauntsaninja deleted the union-perf branch April 23, 2023 22:24
wesleywright pushed a commit that referenced this pull request Apr 24, 2023
This is a performance optimisation for subtyping between two unions that
are largely the same.

Fixes #14034

This makes @adriangb's example in
#14034 (comment)
finish basically instantly. I could add it as a unit test?

Type checking pydantic core is still not fast — takes like four or five
minutes with uncompiled mypy — but at least it's now feasible. I think
there's room for doing some optimisation in make_simplified_union that
would improve this.
hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request Apr 25, 2023
The following code optimises make_simplified_union in the common case
that there are exact duplicates in the union. In this regard, this is
similar to python#15104

To get this to work, I needed to use partial tuple fallbacks in a couple
places (these maybe had the potential to be latent crashes anyway?)
There were some interesting things going on with recursive type aliases
and type state assumptions

This is about a 25% speedup on the pydantic codebase and about a 2%
speedup on self check (measured with uncompiled mypy)
hauntsaninja added a commit that referenced this pull request May 6, 2023
Fixes #15192

The following code optimises make_simplified_union in the common case
that there are exact duplicates in the union. In this regard, this is
similar to #15104

There's a behaviour change in one unit test. I think it's good? We'll
see what mypy_primer has to say.

To get this to work, I needed to use partial tuple fallbacks in a couple
places. These could cause crashes anyway.
There were some interesting things going on with recursive type aliases
and type state assumptions

This is about a 25% speedup on the pydantic codebase and about a 2%
speedup on self check (measured with uncompiled mypy)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mypy 0.990 is 1000x slow on pydantic codebase vs 0.982
5 participants