gh-85160: Reduce memory usage of singledispatchmethod when owner instances cannot be weakref'd#107706
Conversation
…stances cannot be weakref'd
singledispatchmethod when instances cannot be weakref'dsingledispatchmethod when owner instances cannot be weakref'd
|
Alternatively, you can get rid of diff --git a/Lib/functools.py b/Lib/functools.py
index 2a8a69b3c5..5622eb6467 100644
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -935,7 +935,6 @@ def __init__(self, func):
self.dispatcher = singledispatch(func)
self.func = func
self._method_cache = weakref.WeakKeyDictionary()
- self._all_weakrefable_instances = True
def register(self, cls, method=None):
"""generic_method.register(cls, func) -> func
@@ -945,11 +944,11 @@ def register(self, cls, method=None):
return self.dispatcher.register(cls, func=method)
def __get__(self, obj, cls=None):
- if self._all_weakrefable_instances:
+ if self._method_cache:
try:
_method = self._method_cache[obj]
except TypeError:
- self._all_weakrefable_instances = False
+ self._method_cache = None
except KeyError:
pass
else:
@@ -963,7 +962,7 @@ def _method(*args, **kwargs):
_method.register = self.register
update_wrapper(_method, self.func)
- if self._all_weakrefable_instances:
+ if self._method_cache is not None:
self._method_cache[obj] = _method
return _methodIt reduces memory even more, and in all cases. |
Very nice. And the performance seems to be the same. The first line of def __get__(self, obj, cls=None):
if self._method_cache is not None: |
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
951a87e to
3d8d4ba
Compare
|
Or you need to catch TypeError when save in the cache: if self._method_cache is not None:
try:
self._method_cache[obj] = _method
except TypeError:
self._method_cache = None |
With this diff applied to my PR branch: Diff--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -945,7 +945,7 @@ def register(self, cls, method=None):
return self.dispatcher.register(cls, func=method)
def __get__(self, obj, cls=None):
- if self._method_cache is not None:
+ if self._method_cache:
try:
_method = self._method_cache[obj]
except TypeError:
@@ -963,8 +963,10 @@ def _method(*args, **kwargs):
_method.register = self.register
update_wrapper(_method, self.func)
- if self._method_cache is not None:
+ try:
self._method_cache[obj] = _method
+ except TypeError:
+ pass
return _methodBenchmark scriptimport pyperf
setup="""
from functools import singledispatch, singledispatchmethod
class Test:
@singledispatchmethod
def go(self, item, arg):
pass
@go.register
def _(self, item: int, arg):
return item + arg
class Slot:
__slots__ = ('a', 'b')
@singledispatchmethod
def go(self, item, arg):
pass
@go.register
def _(self, item: int, arg):
return item + arg
t = Test()
s= Slot()
"""
runner = pyperf.Runner()
runner.timeit(name="bench singledispatchmethod", stmt="""_ = t.go(1, 1)""", setup=setup )
runner.timeit(name="bench singledispatchmethod slots", stmt="""_ = s.go(1, 1)""", setup=setup ) |
|
Simpler |
I mean that you keep |
I still measure about a 14% overhead from this approach compared to my current PR. (Tested on a PGO-optimised non-debug build on Windows, with a quiet machine.) I'm inclined to stick with how it is currently -- it also feels like slightly cleaner code if we have only one |
With the test for Anyway, your current code is better. I just explained why I intentionally did not use |
Yep. I'm also surprised there was that difference. |
|
Thanks for the review, Serhiy! |
No news, since this is just a small followup to #107148, which is a new feature in Python 3.13
See #107148 (comment) (cc. @eendebakpt)