gh-85160: Reduce memory usage of singledispatchmethod when owner instances cannot be weakref'd

[AlexWaygood]

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)

• Issue: singledispatchmethod significantly slower than singledispatch #85160

[serhiy-storchaka]

Alternatively, you can get rid of self._all_weakrefable_instances:

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 _method

It reduces memory even more, and in all cases.

[AlexWaygood]

Alternatively, you can get rid of self._all_weakrefable_instances:

Very nice. And the performance seems to be the same. The first line of __get__ needs to be this, however (rather than just if self._method_cache), or tests start to fail:

    def __get__(self, obj, cls=None):
        if self._method_cache is not None:

[serhiy-storchaka]

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

[AlexWaygood]

Or you need to catch TypeError when save in the cache:

try/except is expensive; this would be much slower. With this PR currently:

bench singledispatchmethod: Mean +- std dev: 875 ns +- 32 ns
bench singledispatchmethod slots: Mean +- std dev: 2.01 us +- 0.06 us

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 _method

bench singledispatchmethod: Mean +- std dev: 1.02 us +- 0.04 us
bench singledispatchmethod slots: Mean +- std dev: 3.32 us +- 0.27 us

Benchmark script

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

[serhiy-storchaka]

Simpler if self._method_cache: makes the first call faster (this is why I wrote this so at first place). But if self._method_cache is not None: may make all following calls marginally faster, although I do not know how large the difference. So perhaps the latter variant is better in long run.

[serhiy-storchaka]

try/except is expensive; this would be much slower.

I mean that you keep if self._method_cache is not None: for setting, but add catch TypeError. It will not add an overhead.

[AlexWaygood]

It will not add an overhead.

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 try/except in __get__

[serhiy-storchaka]

I still measure about a 14% overhead from this approach compared to my current PR.

With the test for None preceding the try block? I am surprised, try/except should not add an overhead if no exception was raised.

Anyway, your current code is better. I just explained why I intentionally did not use is None in the initial proposition.

[AlexWaygood]

I still measure about a 14% overhead from this approach compared to my current PR.

With the test for None preceding the try block? I am surprised, try/except should not add an overhead if no exception was raised.

Yep. I'm also surprised there was that difference.

[AlexWaygood]

Thanks for the review, Serhiy!