GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-85160: Optimized singledispatchmethod access (noticeable improvement). by mental32 · Pull Request #23213 · python/cpython

Conversation

@mental32

@mental32 mental32 commented Nov 10, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I benchmarked using the following code:

    from functools import singledispatch, singledispatchmethod
    import timeit

    class Test:
        @singledispatchmethod
        def go(self, item, arg):
            print('general')
        
        @go.register
        def _(self, item:int, arg):
            return item + arg

    @singledispatch
    def go(item, arg):
        print('general')

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

    print('m', timeit.timeit('t.go(1, 1)', globals={'t': Test()}))
    print('s', timeit.timeit('go(1, 1)', globals={'go': go}))

And got the following output:

mental@felix ~/D/o/cpython (master)> ./python /tmp/fdsm.py
m 4.794188453000061
s 0.9868643390000216
mental@felix ~/D/o/cpython (master)> git checkout 40988
Switched to branch '40988'
mental@felix ~/D/o/cpython (40988) [1]> ./python /tmp/fdsm.py
m 1.3791182540007867
s 0.9882650030003788

I'm still curious if access could be further optimized clueless when it comes to ideas 😅

https://bugs.python.org/issue40988

  • Issue: singledispatchmethod significantly slower than singledispatch #85160

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mental32
bpo-40988: Optimized singledispatchmethod access.
04fe64e
@mental32
mental32 requested a review from rhettinger as a code owner November 10, 2020 05:12
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Nov 10, 2020
@bedevere-bot bedevere-bot added the awaiting review label Nov 10, 2020
@mental32 mental32 changed the title bpo-40988: Optimized singledispatchmethod access (3x ish improvement). bpo-40988: Optimized singledispatchmethod access (noticeable improvement). Nov 10, 2020
@CaselIT

CaselIT commented Nov 10, 2020

Copy link
Copy Markdown

Thanks for taking this up!
mental32 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Dec 16, 2020

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Dec 16, 2020
@CaselIT

CaselIT commented Dec 16, 2020

Copy link
Copy Markdown

Comment to avoid closure
mental32 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Dec 17, 2020
@github-actions

github-actions Bot commented Jan 16, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jan 16, 2021
@mental32

mental32 commented Jan 16, 2021

Copy link
Copy Markdown
Contributor Author

@rhettinger sorry for bothering you but do you have enough bandwidth to review this or can you defer to someone else?
CaselIT reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jan 17, 2021
@CaselIT

CaselIT commented Sep 17, 2021

Copy link
Copy Markdown

Not sure if there is anything we should do to allow this to be reviewed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rhettinger
rhettinger removed their request for review May 3, 2022 06:14
MaxwellDupre
MaxwellDupre approved these changes May 14, 2022
View reviewed changes

@MaxwellDupre MaxwellDupre left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

On my slow system more modest, but an improvement:
11.24
2.27
to
5.83
3.54
I not that this is built on 3.10.0a2+ so there maybe further speed improvements when 3.11/3.12 is released. So a worthwhile change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels May 14, 2022
@TeamSpen210

TeamSpen210 commented May 14, 2022

Copy link
Copy Markdown

Since this is something that’ll have observable effects on objects, should this be documented? If you’re introspecting your objects, or are say concerned about memory usage it might be surprising behaviour.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@eendebakpt eendebakpt mentioned this pull request Aug 31, 2022
Triage the open "performance" issues on cpython repo faster-cpython/ideas#440
Closed
@iritkatriel iritkatriel added the performance Performance or resource usage label Aug 31, 2022
@CaselIT CaselIT mannequin mentioned this pull request Sep 9, 2022
singledispatchmethod significantly slower than singledispatch #85160
Closed
@AlexWaygood
Merge branch 'main' into bpo-40988
fc72bea
@AlexWaygood AlexWaygood changed the title bpo-40988: Optimized singledispatchmethod access (noticeable improvement). gh-85160: Optimized singledispatchmethod access (noticeable improvement). Jul 5, 2023
@AlexWaygood AlexWaygood mentioned this pull request Jul 5, 2023
gh-85160: improve performance of singledispatchmethod #106448
Closed
@eendebakpt

eendebakpt commented Jul 5, 2023

Copy link
Copy Markdown
Contributor

@mental32 @AlexWaygood By accident I created an independent implementation of the singledispatchmethod improvement. In #106448 I added a performance comparison of this PR and the other one. I also added a test for the case of a class with slots (commit 4e068d4). Feel free to pull that one into this branch.

Both branches give a performance improvement over current main. Hopefully one of them gets accepted.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
eendebakpt
eendebakpt reviewed Jul 5, 2023
View reviewed changes
Comment thread Lib/functools.py
Comment on lines +957 to +963
if self.attrname is not None:
attrname = self.attrname

try:
obj.__dict__[attrname] = _method
except AttributeError:
pass # not all objects have __dict__ (e.g. class defines slots)

@eendebakpt eendebakpt Jul 5, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
if self.attrname is not None:
attrname = self.attrname
try:
obj.__dict__[attrname] = _method
except AttributeError:
pass # not all objects have __dict__ (e.g. class defines slots)
if self.attrname is not None:
try:
obj.__dict__[self.attrname] = _method
except AttributeError:
pass # not all objects have __dict__ (e.g. class defines slots)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@corona10 corona10 self-assigned this Jul 21, 2023
@eendebakpt eendebakpt mentioned this pull request Jul 23, 2023
gh-85160: improve performance of singledispatchmethod #107148
Merged
@mental32

mental32 commented Jul 26, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@eendebakpt thank you for continuing with this!

for any changes or merging that you would like to see done is it okay for me to say "do whatever you need to do :)" and defer to you? (including closing this PR if the other one is sufficient in perf improvements) I don't think I have the time to pay attention to this is all
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood

AlexWaygood commented Aug 6, 2023

Copy link
Copy Markdown
Member

Closing in favour of #107148 (in which you have been listed as a co-author :)
mental32 reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood closed this Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpt eendebakpt eendebakpt left review comments
+1 more reviewer
@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@corona10 corona10

Labels

awaiting core review performance Performance or resource usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@mental32 @CaselIT @TeamSpen210 @eendebakpt @AlexWaygood @MaxwellDupre @iritkatriel @corona10 @the-knights-who-say-ni @ezio-melotti @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.