8000 gh-85160: improve performance of singledispatchmethod by eendebakpt · Pull Request #107148 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-85160: improve performance of singledispatchmethod #107148

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.

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 32 commits into from
Aug 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
04fe64e
bpo-40988: Optimized singledispatchmethod access.
mental32 Nov 10, 2020
fc72bea
Merge branch 'main' into bpo-40988
AlexWaygood Jul 5, 2023
31d292b
Merge branch 'main' into singledispatchmethod3
eendebakpt Jul 23, 2023
5ae8348
add test for slotted classes
eendebakpt Jul 5, 2023
1edc0e3
add test for slotted classes
eendebakpt Jul 23, 2023
9a1ffae
update new entry with attribution to original author
eendebakpt Jul 23, 2023
3588943
add test for assignment to dispatched methods
eendebakpt Jul 23, 2023
af233d4
typo
eendebakpt Jul 23, 2023
d5aebdf
Update Lib/test/test_functools.py
eendebakpt Jul 23, 2023
b773f82
apply review suggestion
eendebakpt Jul 23, 2023
25e1915
Merge branch 'main' into singledispatchmethod3
eendebakpt Jul 23, 2023
593c923
Update Lib/functools.py
eendebakpt Jul 23, 2023
39bec6d
Update Lib/functools.py
eendebakpt Jul 23, 2023
d954244
use hasattr to check for slotted types
eendebakpt Jul 24, 2023
df2eeb5
make attrname private
eendebakpt Jul 24, 2023
774ec3c
review comments
eendebakpt Jul 24, 2023
f03ebba
use weakref for caching
eendebakpt Jul 30, 2023
b459718
Merge branch 'singledispatchmethod3' of githubeendebakpt:eendebakpt/c…
eendebakpt Jul 30, 2023
a422989
Merge branch 'main' into singledispatchmethod3
eendebakpt Jul 30, 2023
450ea12
make patchcheck
eendebakpt Jul 31, 2023
032ab24
review comments
eendebakpt Jul 31, 2023
18d7e43
Apply suggestions from code review
eendebakpt Jul 31, 2023
755efdd
Merge branch 'main' into singledispatchmethod3
eendebakpt Jul 31, 2023
56d342c
Ensure we only weakref() `obj` once
AlexWaygood Jul 31, 2023
a119e21
Get rid of the cache local variable
AlexWaygood Jul 31, 2023
47c18a5
Don't assign `caching` in the fast path
AlexWaygood Jul 31, 2023
2031680
Merge pull request #5 from AlexWaygood/even-more-singledispatchmethod…
eendebakpt Jul 31, 2023
ca691e8
remove check on obj being None
eendebakpt Jul 31, 2023
e8eeedd
eliminate caching variable
eendebakpt Jul 31, 2023
307f0c1
Merge pull request #6 from eendebakpt/singledispatchmethod3b
eendebakpt Aug 1, 2023
afea7b9
Test slotted staticmethods on instances as well as the class
AlexWaygood Aug 1, 2023
b43dcee
eliminate method variable and reduce attribute lookups
eendebakpt Aug 1, 2023
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
21 changes: 19 additions & 2 deletions Lib/functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -928,11 +928,14 @@ class singledispatchmethod:
"""

def __init__(self, func):
import weakref # see comment in singledispatch function
if not callable(func) and not hasattr(func, "__get__"):
raise TypeError(f"{func!r} is not callable or a descriptor")

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
Expand All @@ -942,13 +945,27 @@ def register(self, cls, method=None):
return self.dispatcher.register(cls, func=method)

def __get__(self, obj, cls=None):
if self._all_weakrefable_instances:
try:
_method = self._method_cache[obj]
except TypeError:
self._all_weakrefable_instances = False
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wonder if this would have been a good idea, to keep memory usage down for the slow path (since in the slow path, we don't really need the WeakKeyDictionary at all)

Suggested change
self._all_weakrefable_instances = False
self._all_weakrefable_instances = False
del self._method_cache

@corona10, what do you think? Does it matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One could even delay the creation of the Weakkeydictionary untill the first invocation of __get__. The code will look a bit odd though, a simple del looks cleaner.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think I prefer the approach with del

except KeyError:
pass
else:
return _method

dispatch = self.dispatcher.dispatch
def _method(*args, **kwargs):
method = self.dispatcher.dispatch(args[0].__class__)
return method.__get__(obj, cls)(*args, **kwargs)
return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs)

_method.__isabstractmethod__ = self.__isabstractmethod__
_method.register = self.register
update_wrapper(_method, self.func)

if self._all_weakrefable_instances:
self._method_cache[obj] = _method

return _method

@property
Expand Down
68 changes: 68 additions & 0 deletions Lib/test/test_functools.py
Original file line number Diff line number Diff line change
Expand Up @@ -2474,6 +2474,74 @@ def _(arg):
self.assertTrue(A.t(''))
self.assertEqual(A.t(0.0), 0.0)

def test_slotted_class(self):
class Slot:
__slots__ = ('a', 'b')
@functools.singledispatchmethod
def go(self, item, arg):
pass

@go.register
def _(self, item: int, arg):
return item + arg

s = Slot()
self.assertEqual(s.go(1, 1), 2)

def test_classmethod_slotted_class(self):
class Slot:
__slots__ = ('a', 'b')
@functools.singledispatchmethod
@classmethod
def go(cls, item, arg):
pass

@go.register
@classmethod
def _(cls, item: int, arg):
return item + arg

s = Slot()
self.assertEqual(s.go(1, 1), 2)
self.assertEqual(Slot.go(1, 1), 2)

def test_staticmethod_slotted_class(self):
class A:
__slots__ = ['a']
@functools.singledispatchmethod
@staticmethod
def t(arg):
return arg
@t.register(int)
@staticmethod
def _(arg):
return isinstance(arg, int)
@t.register(str)
@staticmethod
def _(arg):
return isinstance(arg, str)
a = A()

self.assertTrue(A.t(0))
self.assertTrue(A.t(''))
self.assertEqual(A.t(0.0), 0.0)
self.assertTrue(a.t(0))
self.assertTrue(a.t(''))
self.assertEqual(a.t(0.0), 0.0)

def test_assignment_behavior(self):
# see gh-106448
class A:
@functools.singledispatchmethod
def t(arg):
return arg

a = A()
a.t.foo = 'bar'
a2 = A()
with self.assertRaises(AttributeError):
a2.t.foo

def test_classmethod_register(self):
class A:
def __init__(self, arg):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Improve performance of :class:`functools.singledispatchmethod` by caching the
generated dispatch wrapper. Optimization suggested by frederico. Patch by
@mental32, Alex Waygood and Pieter Eendebak.
0