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

typeops: extend make_simplified_union fast path to enums #9394

Merged
merged 2 commits into from
Jul 7, 2021

Conversation

huguesb
Copy link
Contributor

@huguesb huguesb commented Sep 1, 2020

In PR #9192 a fast path was created to address the slowness reported
in issue #9169 wherein large Union or literal types would dramatically
slow down typechecking.

It is desirable to extend this fast path to cover Enum types, as these
can also leverage the O(n) set-based fast path instead of the O(n**2)
fallback.

This is seen to bring down the typechecking of a single fairly simple
chain of if statements operating on a large enum (~3k members) from
~40min to 12s in real-world code! Note that the timing is taken from
a pure-python run of mypy, as opposed to a compiled version.

@huguesb huguesb force-pushed the pr-faster branch 4 times, most recently from 129b2b5 to 57ce8eb Compare September 3, 2020 00:41
In PR python#9192 a fast path was created to address the slowness reported
in issue python#9169 wherein large Union or literal types would dramatically
slow down typechecking.

It is desirable to extend this fast path to cover Enum types, as these
can also leverage the O(n) set-based fast path instead of the O(n**2)
fallback.

This is seen to bring down the typechecking of a single fairly simple
chain of `if` statements operating on a large enum (~3k members) from
~40min to 12s in real-world code! Note that the timing is taken from
a pure-python run of mypy, as opposed to a compiled version.
@huguesb
Copy link
Contributor Author

huguesb commented Sep 15, 2020

Ping, can I get a review?

For typechecking our entire codebase, this single change brings the mypy runtime from 12min down to 5min!

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

This is a fairly complex algorithm (as can be inferred from your comments and local commit history :-).

Can you add some tests that show the correct result is obtained in various edge cases?

Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

This looks pretty plausible to me.

👍 on Guido's comment. A few targeted tests would help here, I think. (Or a convincing argument that the existing test suite covers this well already, I suppose?)

@huguesb
Copy link
Contributor Author

huguesb commented Oct 18, 2020

I believe the existing test suite covers this code very well. It definitely caught the mistake I originally made of skipping the slow path entirely for literals (which breaks down when strict optional is false as indicated in the comment). And since there are no intended behavior changes, it's unclear to me what new tests to add.

@bwo
Copy link
Contributor

bwo commented Jun 7, 2021

ping on this—does hugues' comment above about the existing test suite not convince?

@bwo
Copy link
Contributor

bwo commented Jun 7, 2021

I see it now has conflicts anyway :/

@bwo
Copy link
Contributor

bwo commented Jun 30, 2021

ping

@msullivan
Copy link
Collaborator

Yeah, OK. I'll merge if someone fixes the conflicts.

@msullivan msullivan merged commit fa28616 into python:master Jul 7, 2021
@msullivan
Copy link
Collaborator

(I just did it)

huguesb pushed a commit to huguesb/mypy that referenced this pull request Jan 21, 2022
As a followup to python#9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.
huguesb pushed a commit to huguesb/mypy that referenced this pull request Jan 21, 2022
As a followup to python#9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.
huguesb pushed a commit to huguesb/mypy that referenced this pull request Apr 17, 2022
As a followup to python#9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.
@huguesb huguesb deleted the pr-faster branch April 18, 2022 05:44
JelleZijlstra pushed a commit that referenced this pull request Apr 18, 2022
As a followup to #9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.
JukkaL pushed a commit that referenced this pull request Apr 20, 2022
As a followup to #9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.
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.

5 participants