Skip to content

Another two micro-optimizations #19633

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ilevkivskyi
Copy link
Member

Here are two things:

  • Make FormalArgument a native class. We create huge amount of these (as callable subtyping is one of the most common subtype checks), and named tuples creation is significantly slower than native classes.
  • Do not call re.match() in a code path of format_type(). This is relatively slow (as it is a py_call()) and it is called in almost every error message. This creates problems for code with many third-party dependencies where these errors are ignored anyway.

FWIW in total these give ~0.5% together (I didn't measure individually, but I guess the most benefit for self-check is from the first one).

Comment on lines +1599 to +1617
class FormalArgument:
def __init__(self, name: str | None, pos: int | None, typ: Type, required: bool) -> None:
self.name = name
self.pos = pos
self.typ = typ
self.required = required

def __eq__(self, other: object) -> bool:
if not isinstance(other, FormalArgument):
return NotImplemented
return (
self.name == other.name
and self.pos == other.pos
and self.typ == other.typ
and self.required == other.required
)

def __hash__(self) -> int:
return hash((self.name, self.pos, self.typ, self.required))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would a dataclass work here? According to the docs, the dataclass decorator is supported for native classes. https://mypyc.readthedocs.io/en/latest/native_classes.html#class-decorators

Not sure what the performance difference is though.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is true that dataclasses are extension, but looking at generated C, their constructor still uses **kwargs (i.e. involves creating a dictionary, which is slow). I didn't make measurements, but I guess dataclasses will be somewhere in between.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would slots=True work? There is also frozen which could be set to True as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not that dictionary :-) The one to pack keyword arguments to constructor. Anyway, just to be sure, I tried that, and (as expected) neither changed the constructor calling logic (which is there I guess because constructor is auto-generated not by us).

This comment has been minimized.

Co-authored-by: Ali Hamdan <[email protected]>
Copy link
Contributor

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

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.

4 participants