From 7c5bb0e8e3e9583fb4d9a4749cf47e29d36512f7 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 11 Feb 2025 18:37:31 +0200 Subject: [PATCH 1/3] gh-127750: Fix and optimize functools.singledispatchmethod() Remove broken singledispatchmethod caching introduced in gh-85160. Achieve the same performance using different optimization. --- Lib/functools.py | 60 ++++++++++--------- Lib/test/test_functools.py | 3 + ...-02-11-18-37-26.gh-issue-127750.41SDhF.rst | 2 + 3 files changed, 38 insertions(+), 27 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-02-11-18-37-26.gh-issue-127750.41SDhF.rst diff --git a/Lib/functools.py b/Lib/functools.py index fd33f0ae479ddc..5491e566499892 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1026,9 +1026,6 @@ def __init__(self, func): self.dispatcher = singledispatch(func) self.func = func - import weakref # see comment in singledispatch function - self._method_cache = weakref.WeakKeyDictionary() - def register(self, cls, method=None): """generic_method.register(cls, func) -> func @@ -1037,36 +1034,45 @@ 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: - try: - _method = self._method_cache[obj] - except TypeError: - self._method_cache = None - except KeyError: - pass - else: - return _method + return _singledispatchmethod_get(self, obj, cls) - dispatch = self.dispatcher.dispatch - funcname = getattr(self.func, '__name__', 'singledispatchmethod method') - def _method(*args, **kwargs): - if not args: - raise TypeError(f'{funcname} requires at least ' - '1 positional argument') - return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs) + @property + def __isabstractmethod__(self): + return getattr(self.func, '__isabstractmethod__', False) - _method.__isabstractmethod__ = self.__isabstractmethod__ - _method.register = self.register - update_wrapper(_method, self.func) - if self._method_cache is not None: - self._method_cache[obj] = _method +class _singledispatchmethod_get: + def __init__(self, unbound, obj, cls): + self._unbound = unbound + self._dispatch = unbound.dispatcher.dispatch + self._obj = obj + self._cls = cls - return _method + def __call__(self, /, *args, **kwargs): + if not args: + funcname = getattr(self._unbound.func, '__name__', + 'singledispatchmethod method') + raise TypeError(f'{funcname} requires at least ' + '1 positional argument') + return self._dispatch(args[0].__class__).__get__(self._obj, self._cls)(*args, **kwargs) + + def __getattr__(self, name): + if name not in {'__name__', '__qualname__', '__isabstractmethod__', + '__annotations__', '__type_params__'}: + raise AttributeError + return getattr(self._unbound.func, name) @property - def __isabstractmethod__(self): - return getattr(self.func, '__isabstractmethod__', False) + def __module__(self): + return self._unbound.func.__module__ + + @property + def __doc__(self): + return self._unbound.func.__doc__ + + @property + def register(self): + return self._unbound.register ################################################################################ diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 1b7a76bec839bf..4cb5e97c024c35 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2900,6 +2900,7 @@ def static_func(arg: int) -> str: """My function docstring""" return str(arg) + prefix = A.__qualname__ + '.' for meth in ( A.func, A().func, @@ -2909,6 +2910,8 @@ def static_func(arg: int) -> str: A().static_func ): with self.subTest(meth=meth): + self.assertEqual(meth.__module__, __name__) + self.assertEqual(meth.__qualname__, prefix + meth.__name__) self.assertEqual(meth.__doc__, ('My function docstring' if support.HAVE_DOCSTRINGS diff --git a/Misc/NEWS.d/next/Library/2025-02-11-18-37-26.gh-issue-127750.41SDhF.rst b/Misc/NEWS.d/next/Library/2025-02-11-18-37-26.gh-issue-127750.41SDhF.rst new file mode 100644 index 00000000000000..b119e296276ac1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-11-18-37-26.gh-issue-127750.41SDhF.rst @@ -0,0 +1,2 @@ +Remove broken :func:`functools.singledispatchmethod` caching introduced in +:gh:`85160`. Achieve the same performance using different optimization. From 93d09772689f99f1b6e244f24568e99f6a18c32f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 11 Feb 2025 20:07:01 +0200 Subject: [PATCH 2/3] Add more tests. --- Lib/test/test_functools.py | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 4cb5e97c024c35..b82b1c4c2a9b71 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -3244,6 +3244,64 @@ def f(arg): def _(arg: undefined): return "forward reference" + def test_method_equal_instances(self): + # gh-127750: Reference to self was cached + class A: + def __eq__(self, other): + return True + def __hash__(self): + return 1 + @functools.singledispatchmethod + def t(self, arg): + return self + + a = A() + b = A() + self.assertIs(a.t(1), a) + self.assertIs(b.t(2), b) + + def test_method_bad_hash(self): + class A: + def __eq__(self, other): + raise AssertionError + def __hash__(self): + raise AssertionError + @functools.singledispatchmethod + def t(self, arg): + pass + + # Should not raise + A().t(1) + hash(A().t) + A().t == A().t + + def test_method_no_reference_loops(self): + # gh-127750: Created a strong reference to self + class A: + @functools.singledispatchmethod + def t(self, arg): + return weakref.ref(self) + + a = A() + r = a.t(1) + self.assertIsNotNone(r()) + del a # delete a after a.t + if not support.check_impl_detail(cpython=True): + support.gc_collect() + self.assertIsNone(r()) + + a = A() + t = a.t + del a # delete a before a.t + support.gc_collect() + r = t(1) + self.assertIsNotNone(r()) + del t + if not support.check_impl_detail(cpython=True): + support.gc_collect() + self.assertIsNone(r()) + + class CachedCostItem: _cost = 1 From 16604a114be893bf48ab0e7a1ad4b3dbe499d719 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 12 Feb 2025 12:37:04 +0200 Subject: [PATCH 3/3] Fix issues with __module__ and __doc__ descriptors. --- Lib/functools.py | 21 +++++++++++++-------- Lib/test/test_functools.py | 1 + 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index 5491e566499892..92be41dcf8e9e3 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1047,6 +1047,17 @@ def __init__(self, unbound, obj, cls): self._dispatch = unbound.dispatcher.dispatch self._obj = obj self._cls = cls + # Set instance attributes which cannot be handled in __getattr__() + # because they conflict with type descriptors. + func = unbound.func + try: + self.__module__ = func.__module__ + except AttributeError: + pass + try: + self.__doc__ = func.__doc__ + except AttributeError: + pass def __call__(self, /, *args, **kwargs): if not args: @@ -1057,19 +1068,13 @@ def __call__(self, /, *args, **kwargs): return self._dispatch(args[0].__class__).__get__(self._obj, self._cls)(*args, **kwargs) def __getattr__(self, name): + # Resolve these attributes lazily to speed up creation of + # the _singledispatchmethod_get instance. if name not in {'__name__', '__qualname__', '__isabstractmethod__', '__annotations__', '__type_params__'}: raise AttributeError return getattr(self._unbound.func, name) - @property - def __module__(self): - return self._unbound.func.__module__ - - @property - def __doc__(self): - return self._unbound.func.__doc__ - @property def register(self): return self._unbound.register diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index b82b1c4c2a9b71..35c6631d908b6e 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -2911,6 +2911,7 @@ def static_func(arg: int) -> str: ): with self.subTest(meth=meth): self.assertEqual(meth.__module__, __name__) + self.assertEqual(type(meth).__module__, 'functools') self.assertEqual(meth.__qualname__, prefix + meth.__name__) self.assertEqual(meth.__doc__, ('My function docstring'