more enum-related speedups #12032
more enum-related speedups #12032
Conversation
mypy/meet.py
Outdated
| return ( | ||
| isinstance(x, Instance) and x.type.is_enum and | ||
| isinstance(y, UnionType) and | ||
| all(isinstance(z, LiteralType) and z.fallback.type == x.type # type: ignore[misc] |
There was a problem hiding this comment.
Why do we need type: ignore here? 🤔
There was a problem hiding this comment.
because when mypy typechecks itself it complains about using isinstance on values of type Type, suggesting running them through get_proper_type first (which I don't think is appropriate here) or using this type: ignore
mypy/meet.py
Outdated
| return ( | ||
| isinstance(x, LiteralType) and isinstance(y, UnionType) and any( | ||
| isinstance(z, LiteralType) and z == x # type: ignore[misc] | ||
| for z in y.items |
mypy/meet.py
Outdated
| # and crucially, we want to do that *fast* in case the enum is large | ||
| # so we do it before expanding variants below to avoid O(n**2) behavior | ||
| if ( | ||
| is_enum_overlapping_union(left, right) or |
There was a problem hiding this comment.
style nit: we use \n or is_enum_overlapping_union(...
mypy/subtypes.py
Outdated
| # avoid redundant check for union of literals | ||
| for item in left.items: | ||
| if mypy.typeops.is_simple_literal(item): | ||
| assert isinstance(item, LiteralType) # type: ignore[misc] |
There was a problem hiding this comment.
It might be a good idea to declare item = get_proper_type(item)
There was a problem hiding this comment.
or I suppose I might be able to use the new TypeGuard feature here?
There was a problem hiding this comment.
Yes 🙂
But, I am not sure that mypyc supports it.
There was a problem hiding this comment.
black is compiled with mypyc and you added TypeGuard to it @sobolevn so it's at least worth a shot imo 🙂
There was a problem hiding this comment.
well OK it turns out I misremembered sobolevn for someone else, but anyway the first half of my still stands 😅
This comment has been minimized.
This comment has been minimized.
|
Any chance to get this merged? |
| @@ -1137,6 +1150,17 @@ def report(*args: Any) -> None: | |||
| return applied | |||
|
|
|||
|
|
|||
| def try_restrict_literal_union(t: UnionType, s: Type) -> Optional[List[Type]]: | |||
There was a problem hiding this comment.
Could you add a docstring to this function?
As a followup to python#9394 address a few more O(n**2) behaviors caused by decomposing enums into unions of literals.
huguesb
force-pushed
the
pr-enum-speedups
branch
from
2aa1337 to
774b95c
Compare
|
Rebased on top of master after recent union-related changes |
This comment has been minimized.
This comment has been minimized.
mypy/meet.py
Outdated
| all(x.type == p.fallback.type | ||
| for p in (get_proper_type(z) for z in y.relevant_items()) | ||
| if isinstance(p, LiteralType)) |
There was a problem hiding this comment.
| all(x.type == p.fallback.type | |
| for p in (get_proper_type(z) for z in y.relevant_items()) | |
| if isinstance(p, LiteralType)) | |
| all(isinstance(p, LiteralType)) and x.type == p.fallback.type | |
| for p in (get_proper_type(z) for z in y.relevant_items())) |
Should it be this instead? As I understand it, this function checks that x is an enum and y is a union containing only members of x.
It would be helpful to add a docstring to this function.
There was a problem hiding this comment.
Upon looking at this again, I think the previous implementation was wrong since we only care about partial overlap. Fixed and documented accordingly.
mypy/sametypes.py
Outdated
| @@ -153,14 +153,32 @@ def visit_literal_type(self, left: LiteralType) -> bool: | |||
|
|
|||
| def visit_union_type(self, left: UnionType) -> bool: | |||
| if isinstance(self.right, UnionType): | |||
| # fast path for simple literals | |||
| def _extract_literals(u: UnionType) -> Tuple[Set[Type], List[Type]]: | |||
There was a problem hiding this comment.
I believe nested functions don't perform well with mypyc, can you make this a global function instead?
mypy/subtypes.py
Outdated
| @@ -1199,6 +1213,18 @@ def report(*args: Any) -> None: | |||
| return applied | |||
|
|
|||
|
|
|||
| def try_restrict_literal_union(t: UnionType, s: Type) -> Optional[List[Type]]: | |||
| """Helper function for restrict_subtype_away, allowing a fast path for Union of simple literals""" | |||
There was a problem hiding this comment.
This docstring doesn't say what the function does, which is the part I was having trouble with. What are the parameters and what does it return?
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
As a followup to #9394 address a few more O(n**2) behaviors caused by decomposing enums into unions of literals.
As a followup to #9394 address a few more O(n**2) behaviors
caused by decomposing enums into unions of literals.