-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Reduce memory usage by using __slots__ #4904
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I think I forgot to send these comments.
mypy/types.py
Outdated
can_be_false = False | ||
__slots__ = ('fallback',) | ||
|
||
MYPY = False # Use this to declare, we don't want a runtime None value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'd rather see MYPY = False
as a global (though the comment belongs in the class).
mypy/types.py
Outdated
MYPY = False # Use this to declare, we don't want a runtime None value | ||
if MYPY: | ||
# Corresponding instance type (e.g. builtins.type) | ||
fallback = None # type: Instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not do this in __init__()
?
mypy/types.py
Outdated
"""Type of a non-overloaded callable object (such as function). | ||
|
||
Attributes: | ||
arg_types: Types of function arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(What a bummer we can't use PEP 526 notation yet. :-( )
mypy/types.py
Outdated
'implicit', | ||
'special_sig', | ||
'from_type_type', | ||
'bound_args') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a trailing comma here. Also perhaps you can add the comments here rather than to the docstring?
mypy/util.py
Outdated
|
||
def replace_object_state(new: object, old: object) -> None: | ||
# Copy state of old node to the new node. This varies depending on whether | ||
# there is __slots__ and/or __dict__. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this should be a docstring, not a comment.
mypy/util.py
Outdated
if hasattr(old, '__dict__'): | ||
new.__dict__ = old.__dict__ | ||
if hasattr(old, '__slots__'): | ||
# Use type.mro(...) since some classes override 'mro' with something different. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or use __mro__
, I doubt they override that.
mypy/util.py
Outdated
new.__dict__ = old.__dict__ | ||
if hasattr(old, '__slots__'): | ||
# Use type.mro(...) since some classes override 'mro' with something different. | ||
for base in type.mro(type(old)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be worth it caching the combined set of slots on the class, so you don't have to walk the MRO each time.
mypy/nodes.py
Outdated
def __init__(self) -> None: | ||
super().__init__() | ||
# Type signature. This is usually CallableType or Overloaded, but it can be | ||
# something else for decorated functions/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the trailing /
mypy/nodes.py
Outdated
'is_awaitable_coroutine', | ||
'is_static', | ||
'is_class', | ||
'expanded') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add trailing comma (and move )
to next line, also in other classes below), and consider adding the comments here rather than to the docstring?
mypy/nodes.py
Outdated
@@ -507,22 +558,23 @@ def is_dynamic(self) -> bool: | |||
return self.type is None | |||
|
|||
|
|||
FUNCDEF_FLAGS = FUNCITEM_FLAGS + [ | |||
'is_decorated', 'is_conditional', 'is_abstract', 'is_property' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing comma.
I think that I've addressed all comments (or at least most). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good now! You have a merge conflict though. :-(
In some cases this reduces the memory usage of mypy and mypy-daemon
over 30%. This seems to have only a minor effect on speed. Depending on
the workload, this seems to either speed up or slow down runtimes by a
few percent.
Notes:
conflict with
__slots__
.__slots__
to classes that use the most memory, basedon the output from the memory profiler.
None
defaults even though the type was notoptional. Now we use a cast for some of these, which has the same
effect.
__dict__
manipulations had to be modified to support__slots__
as well.