Skip to content

Optimize ABC caches #383

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.

Sign up for GitHub

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 8 commits into from
Feb 10, 2017
Merged

Optimize ABC caches #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
313ca43
Optimize ABC negative cache for generics
Feb 1, 2017
6382c8a
Add also positive cache and registry links
Feb 1, 2017
6d1bcf2
Formatting
Feb 1, 2017
c464731
Add ABC optimization to PY2
Feb 1, 2017
423fb75
Inline internal helper for more speed
Feb 1, 2017
f998e24
Fix negative cahce setters
ilevkivskyi Feb 2, 2017
9444b53
Fix negative cahce setters on PY2
ilevkivskyi Feb 2, 2017
0ca5c0b
Make negative caches writable only for non-subscripted generics
ilevkivskyi Feb 2, 2017
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
65 changes: 48 additions & 17 deletions python2/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,19 +928,6 @@ def _next_in_mro(cls):
return next_in_mro


def _valid_for_check(cls):
"""An internal helper to prohibit isinstance([1], List[str]) etc."""
if cls is Generic:
raise TypeError("Class %r cannot be used with class "
"or instance checks" % cls)
if (
cls.__origin__ is not None and
sys._getframe(3).f_globals['__name__'] not in ['abc', 'functools']
):
raise TypeError("Parameterized generics cannot be used with class "
"or instance checks")


def _make_subclasshook(cls):
"""Construct a __subclasshook__ callable that incorporates
the associated __extra__ class in subclass checks performed
Expand All @@ -951,7 +938,6 @@ def _make_subclasshook(cls):
# Registered classes need not be checked here because
# cls and its extra share the same _abc_registry.
def __extrahook__(cls, subclass):
_valid_for_check(cls)
res = cls.__extra__.__subclasshook__(subclass)
if res is not NotImplemented:
return res
Expand All @@ -966,7 +952,6 @@ def __extrahook__(cls, subclass):
else:
# For non-ABC extras we'll just call issubclass().
def __extrahook__(cls, subclass):
_valid_for_check(cls)
if cls.__extra__ and issubclass(subclass, cls.__extra__):
return True
return NotImplemented
Expand Down Expand Up @@ -1044,6 +1029,7 @@ def __new__(cls, name, bases, namespace,
# remove bare Generic from bases if there are other generic bases
if any(isinstance(b, GenericMeta) and b is not Generic for b in bases):
bases = tuple(b for b in bases if b is not Generic)
namespace.update({'__origin__': origin, '__extra__': extra})
self = super(GenericMeta, cls).__new__(cls, name, bases, namespace)

self.__parameters__ = tvars
Expand All @@ -1052,8 +1038,6 @@ def __new__(cls, name, bases, namespace,
self.__args__ = tuple(Ellipsis if a is _TypingEllipsis else
() if a is _TypingEmpty else
a for a in args) if args else None
self.__origin__ = origin
self.__extra__ = extra
# Speed hack (https://github.com/python/typing/issues/196).
self.__next_in_mro__ = _next_in_mro(self)
# Preserve base classes on subclassing (__bases__ are type erased now).
Expand Down Expand Up @@ -1081,6 +1065,42 @@ def __init__(self, *args, **kwargs):
super(GenericMeta, self).__init__(*args, **kwargs)
if isinstance(self.__extra__, abc.ABCMeta):
self._abc_registry = self.__extra__._abc_registry
self._abc_cache = self.__extra__._abc_cache
elif self.__origin__ is not None:
self._abc_registry = self.__origin__._abc_registry
self._abc_cache = self.__origin__._abc_cache
Copy link
Member

Choose a reason for hiding this comment

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

How about sharing self._abc_negative_cache too?

Copy link
Member Author

@ilevkivskyi ilevkivskyi Feb 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

self._abc_negative_cache is also shared, but it could be overwritten by abc.py on negative cache invalidation, this is why I made it a descriptor 7 lines below. (Positive cache is never invalidated, so that the sharing could be realised as a simple assignment on instantiation).

Copy link
Member

Choose a reason for hiding this comment

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

tracemalloc shows this.
I think removing _abc_negative_cache at __init__ or __new__ is worth enough.

125.34KiB / count=573
  File "/Users/inada-n/local/py37/lib/python3.7/_weakrefset.py", line 48
    self._iterating = set()
  File "/Users/inada-n/local/py37/lib/python3.7/abc.py", line 147
    cls._abc_negative_cache = WeakSet()
  File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 125
    return super().__new__(cls, name, bases, namespace)

123.44KiB / count=1193
  File "/Users/inada-n/local/py37/lib/python3.7/_weakrefset.py", line 38
    def _remove(item, selfref=ref(self)):
  File "/Users/inada-n/local/py37/lib/python3.7/abc.py", line 147
    cls._abc_negative_cache = WeakSet()
  File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 125
    return super().__new__(cls, name, bases, namespace)

Copy link
Member Author

Choose a reason for hiding this comment

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

I could not just remove them, since some other places expect their presence. However, I found a bug in _abc_negative_cache setters, and I made most assignments to _abc_negative_cache a no-op. Could you please check the memory/speed situation once more with the newest commit in my branch?


# _abc_negative_cache and _abc_negative_cache_version
# realised as descriptors, since GenClass[t1, t2, ...] always
# share subclass info with GenClass.
# This is an important memory optimization.
@property
def _abc_negative_cache(self):
if isinstance(self.__extra__, abc.ABCMeta):
return self.__extra__._abc_negative_cache
return _gorg(self)._abc_generic_negative_cache

@_abc_negative_cache.setter
def _abc_negative_cache(self, value):
if self.__origin__ is None:
if isinstance(self.__extra__, abc.ABCMeta):
self.__extra__._abc_negative_cache = value
else:
self._abc_generic_negative_cache = value

@property
def _abc_negative_cache_version(self):
if isinstance(self.__extra__, abc.ABCMeta):
return self.__extra__._abc_negative_cache_version
return _gorg(self)._abc_generic_negative_cache_version

@_abc_negative_cache_version.setter
def _abc_negative_cache_version(self, value):
if self.__origin__ is None:
if isinstance(self.__extra__, abc.ABCMeta):
self.__extra__._abc_negative_cache_version = value
else:
self._abc_generic_negative_cache_version = value

def _get_type_vars(self, tvars):
if self.__origin__ and self.__parameters__:
Expand Down Expand Up @@ -1180,6 +1200,17 @@ def __getitem__(self, params):
extra=self.__extra__,
orig_bases=self.__orig_bases__)

def __subclasscheck__(self, cls):
if self.__origin__ is not None:
if sys._getframe(1).f_globals['__name__'] not in ['abc', 'functools']:
raise TypeError("Parameterized generics cannot be used with class "
"or instance checks")
return False
if self is Generic:
raise TypeError("Class %r cannot be used with class "
"or instance checks" % self)
return super(GenericMeta, self).__subclasscheck__(cls)

def __instancecheck__(self, instance):
# Since we extend ABC.__subclasscheck__ and
# ABC.__instancecheck__ inlines the cache checking done by the
Expand Down
65 changes: 48 additions & 17 deletions src/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,19 +849,6 @@ def _next_in_mro(cls):
return next_in_mro


def _valid_for_check(cls):
"""An internal helper to prohibit isinstance([1], List[str]) etc."""
if cls is Generic:
raise TypeError("Class %r cannot be used with class "
"or instance checks" % cls)
if (
cls.__origin__ is not None and
sys._getframe(3).f_globals['__name__'] not in ['abc', 'functools']
):
raise TypeError("Parameterized generics cannot be used with class "
"or instance checks")


def _make_subclasshook(cls):
"""Construct a __subclasshook__ callable that incorporates
the associated __extra__ class in subclass checks performed
Expand All @@ -872,7 +859,6 @@ def _make_subclasshook(cls):
# Registered classes need not be checked here because
# cls and its extra share the same _abc_registry.
def __extrahook__(subclass):
_valid_for_check(cls)
res = cls.__extra__.__subclasshook__(subclass)
if res is not NotImplemented:
return res
Expand All @@ -887,7 +873,6 @@ def __extrahook__(subclass):
else:
# For non-ABC extras we'll just call issubclass().
def __extrahook__(subclass):
_valid_for_check(cls)
if cls.__extra__ and issubclass(subclass, cls.__extra__):
return True
return NotImplemented
Expand Down Expand Up @@ -974,6 +959,7 @@ def __new__(cls, name, bases, namespace,
# remove bare Generic from bases if there are other generic bases
if any(isinstance(b, GenericMeta) and b is not Generic for b in bases):
bases = tuple(b for b in bases if b is not Generic)
namespace.update({'__origin__': origin, '__extra__': extra})
self = super().__new__(cls, name, bases, namespace, _root=True)

self.__parameters__ = tvars
Expand All @@ -982,8 +968,6 @@ def __new__(cls, name, bases, namespace,
self.__args__ = tuple(... if a is _TypingEllipsis else
() if a is _TypingEmpty else
a for a in args) if args else None
self.__origin__ = origin
self.__extra__ = extra
# Speed hack (https://github.com/python/typing/issues/196).
self.__next_in_mro__ = _next_in_mro(self)
# Preserve base classes on subclassing (__bases__ are type erased now).
Expand All @@ -1002,13 +986,49 @@ def __new__(cls, name, bases, namespace,
self.__subclasshook__ = _make_subclasshook(self)
if isinstance(extra, abc.ABCMeta):
self._abc_registry = extra._abc_registry
self._abc_cache = extra._abc_cache
elif origin is not None:
self._abc_registry = origin._abc_registry
self._abc_cache = origin._abc_cache

if origin and hasattr(origin, '__qualname__'): # Fix for Python 3.2.
self.__qualname__ = origin.__qualname__
self.__tree_hash__ = (hash(self._subs_tree()) if origin else
super(GenericMeta, self).__hash__())
return self

# _abc_negative_cache and _abc_negative_cache_version
# realised as descriptors, since GenClass[t1, t2, ...] always
# share subclass info with GenClass.
# This is an important memory optimization.
@property
def _abc_negative_cache(self):
if isinstance(self.__extra__, abc.ABCMeta):
return self.__extra__._abc_negative_cache
return _gorg(self)._abc_generic_negative_cache

@_abc_negative_cache.setter
def _abc_negative_cache(self, value):
if self.__origin__ is None:
if isinstance(self.__extra__, abc.ABCMeta):
self.__extra__._abc_negative_cache = value
else:
self._abc_generic_negative_cache = value

@property
def _abc_negative_cache_version(self):
if isinstance(self.__extra__, abc.ABCMeta):
return self.__extra__._abc_negative_cache_version
return _gorg(self)._abc_generic_negative_cache_version

@_abc_negative_cache_version.setter
def _abc_negative_cache_version(self, value):
if self.__origin__ is None:
if isinstance(self.__extra__, abc.ABCMeta):
self.__extra__._abc_negative_cache_version = value
else:
self._abc_generic_negative_cache_version = value

def _get_type_vars(self, tvars):
if self.__origin__ and self.__parameters__:
_get_type_vars(self.__parameters__, tvars)
Expand Down Expand Up @@ -1107,6 +1127,17 @@ def __getitem__(self, params):
extra=self.__extra__,
orig_bases=self.__orig_bases__)

def __subclasscheck__(self, cls):
if self.__origin__ is not None:
if sys._getframe(1).f_globals['__name__'] not in ['abc', 'functools']:
raise TypeError("Parameterized generics cannot be used with class "
"or instance checks")
return False
if self is Generic:
raise TypeError("Class %r cannot be used with class "
"or instance checks" % self)
return super().__subclasscheck__(cls)

def __instancecheck__(self, instance):
# Since we extend ABC.__subclasscheck__ and
# ABC.__instancecheck__ inlines the cache checking done by the
Expand Down