Skip to content

Micro-optimize flatten_nested_unions #14368

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

Merged
merged 1 commit into from
Dec 29, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -3381,12 +3381,20 @@ def _flattened(types: Iterable[Type]) -> Iterable[Type]:


def flatten_nested_unions(
types: Iterable[Type], handle_type_alias_type: bool = True
types: Sequence[Type], handle_type_alias_type: bool = True
) -> list[Type]:
"""Flatten nested unions in a type list."""
if not isinstance(types, list):
typelist = list(types)
else:
typelist = cast("list[Type]", types)

# Fast path: most of the time there is nothing to flatten
if not any(isinstance(t, (TypeAliasType, UnionType)) for t in typelist): # type: ignore[misc]
return typelist
Comment on lines +3387 to +3394
Copy link
Member

Choose a reason for hiding this comment

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

I think we only need to convert it to a list if we're taking the fast path, correct?

Suggested change
if not isinstance(types, list):
typelist = list(types)
else:
typelist = cast("list[Type]", types)
# Fast path: most of the time there is nothing to flatten
if not any(isinstance(t, (TypeAliasType, UnionType)) for t in typelist): # type: ignore[misc]
return typelist
# Fast path: most of the time there is nothing to flatten
if not any(isinstance(t, (TypeAliasType, UnionType)) for t in types): # type: ignore[misc]
if not isinstance(types, list):
return list(types)
return cast("list[Type]", types)

Copy link
Collaborator Author

@JukkaL JukkaL Dec 29, 2022

Choose a reason for hiding this comment

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

I'm first converting it to a list since iterating over a list if faster than iterating over a sequence, and since we need to return a list anyway, so it would only delay the construction of the list. Since we take the fast path the vast majority of the time, the redundant construction of a list in the slow path is not a big deal, and we save some of the cost through faster iteration. If mypyc was smarter about iterating over sequences, your suggested approach would be better.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation!


flat_items: list[Type] = []
# TODO: avoid duplicate types in unions (e.g. using hash)
for t in types:
for t in typelist:
tp = get_proper_type(t) if handle_type_alias_type else t
if isinstance(tp, ProperType) and isinstance(tp, UnionType):
flat_items.extend(
Expand Down