Optimize ABC caches#383

ilevkivskyi

Extensive use of isinstance with ABCs causes significant increase in memory consumption by generics, see https://mail.python.org/pipermail/python-dev/2017-January/147194.html

Here I make _abc_cache and friends shortcut to the type's __extra__ or _gorg (using descriptors where necessary). This allows to reduce memory consumption up to 4x on a micro-benchmark simulating conditions described in the python-dev thread. (There is of course a speed penalty, but it seems to be minor).

@methane Please take a look.

ilevkivskyi

@methane I also inlined internal helpers to speed-up isinstance() up to 2x.

methane

Because abc is full of magic, I can't say LGTM.
But basic idea seems very right. Thanks to fix it.
I'll check startup time and memory consumption.

BTW, this issue is new from 3.5.3 and 3.6.0?
Does Python 3.5.2 have less ABCs when using lot's of typing.List[X] for hundreds of classes?

How about sharing self._abc_negative_cache too?

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).

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)

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?

ilevkivskyi

@methane

BTW, this issue is new from 3.5.3 and 3.6.0?

I think it was always like this. Actually, it was maybe even worse. There were some improvements in 3.5.3 and 3.6.0 due to generic type caches (these are unrelated caches that actually help :-))

methane

Memory consumption goes wrong.

996.90KiB / count=3450 File "/Users/inada-n/local/py37/lib/python3.7/abc.py", line 133 cls = super().__new__(mcls, name, bases, namespace) File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 125 return super().__new__(cls, name, bases, namespace) File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 963 self = super().__new__(cls, name, bases, namespace, _root=True) File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 1126 orig_bases=self.__orig_bases__) File "/Users/inada-n/local/py37/lib/python3.7/typing.py", line 642 return cached(*args, **kwds)

methane

I can reproduce above with original typing.py.

It wasn't in https://gist.github.com/methane/3c34f11fb677365a7e92afe73aca24e7
So it may caused by some internal mechanizm.

("", line 488 usually includes memory for
intern dict. Some internal cache affects tracemalloc.)

ilevkivskyi

The last trace you posted (with 996.90KiB / count=3450) seems to be unrelated to _abc_caches. This is normal creation of generic classes. It could be a problem with tracemalloc. If necessary, you could clean-up generic type caches:

for c in typing._cleanups: c()

I don't know how to clean-up other internal caches.

Could you please compare the master version and my patch both in same conditions "from cold start"?

If you are interested, I am running a "micro-benchmark":

import sys
from typing import *
from collections.abc import Iterable

classes = []
for i in range(1000):
    classes.append(type('C'+str(i), (), {}))
    dummy = List[classes[i]]

for i in range(1000):
    dummy = isinstance(classes[i](), Iterable)

sys._debugmallocstats()

This gives me total memory consumption 5.5MB with my patch vs 48MB for typing/master.

methane

As I said above, i can reproduce it with default version of typing. There are no regressions. 2017/02/02 午後10:44 "Ivan Levkivskyi" <notifications@github.com>:

…

The last trace you posted (with 996.90KiB / count=3450) seems to be unrelated to _abc_caches. This is normal creation of generic classes. It could be a problem with tracemalloc. If necessary, you could clean-up generic type caches: for c in typing._cleanups: c() I don't know how to clean-up other internal caches. Could you please compare the master version and my patch both in same conditions "from cold start"? ------------------------------ If you are interested, I am running a "micro-benchmark": import sysfrom typing import *from collections.abc import Iterable classes = []for i in range(1000): classes.append(type('C'+str(i), (), {})) dummy = List[classes[i]] for i in range(1000): dummy = isinstance(classes[i](), Iterable) sys._debugmallocstats() This gives me total memory consumption 5.5MB with my patch vs 48MB for typing/master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#383 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMLqOlWiR55Oauh63T5OXsuof73RMjMks5rYd3DgaJpZM4Lz-42> .

ilevkivskyi

There are no regressions.

OK, thanks! Do you think the PR is satisfactory?
We could think about other possible optimizations later.

methane

It looks nice to me.
But I'm not wizard of metaclass. I'm not good code reviewer of this module.

ilevkivskyi

@methane
Thank you, it was very helpful to see the benchmark on a large code base.

gvanrossum

I continue to lack the energy to review/approve this (I can handle only moderately complex reviews ATM). Given that the 3.6.1 candidate is expected in about two weeks (see PEP 494) maybe the best way forward is to merge this (and the coverage PR that depends on it) no, merge everything upstream, and hope for the best? The memory stats look encouraging for sure!

ilevkivskyi

OK, let's do this. After merging, I will make additional checks (I do refleak checks from time to time, and run full CPython test suite after importing typing).

the-knights-who-say-ni added the CLA signed label Feb 1, 2017

methane reviewed Feb 2, 2017

View reviewed changes

ilevkivskyi mentioned this pull request Feb 3, 2017

Add some tests based on coverage report: Part 2 #375

Merged

Ivan Levkivskyi and others added 8 commits February 4, 2017 11:03

Optimize ABC negative cache for generics

313ca43

Add also positive cache and registry links

6382c8a

Formatting

6d1bcf2

Add ABC optimization to PY2

c464731

Inline internal helper for more speed

423fb75

Fix negative cahce setters

f998e24

Fix negative cahce setters on PY2

9444b53

Make negative caches writable only for non-subscripted generics

0ca5c0b

ilevkivskyi force-pushed the optimize-weakset branch from 149b538 to 0ca5c0b Compare February 4, 2017 10:12

ilevkivskyi merged commit 58c86ad into python:master Feb 10, 2017

ilevkivskyi mentioned this pull request Sep 3, 2017

Proposal: Make generic types non-classes. #468

Closed

+                          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

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Optimize ABC caches#383