Skip to content
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

Could operator.methodcaller be optimized using LOAD_METHOD? #89013

Closed
anntzer mannequin opened this issue Aug 6, 2021 · 5 comments
Closed

Could operator.methodcaller be optimized using LOAD_METHOD? #89013

anntzer mannequin opened this issue Aug 6, 2021 · 5 comments
Labels
3.12 bugs and security fixes performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@anntzer
Copy link
Mannequin

anntzer mannequin commented Aug 6, 2021

BPO 44850
Nosy @markshannon, @anntzer, @sweeneyde
PRs
  • bpo-44850: Improve the performance of methodcaller. #27782
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2021-08-06.10:42:14.018>
    labels = ['3.11', 'library', 'performance']
    title = 'Could operator.methodcaller be optimized using LOAD_METHOD?'
    updated_at = <Date 2021-08-19.10:07:47.016>
    user = 'https://github.com/anntzer'

    bugs.python.org fields:

    activity = <Date 2021-08-19.10:07:47.016>
    actor = 'Mark.Shannon'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2021-08-06.10:42:14.018>
    creator = 'Antony.Lee'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44850
    keywords = ['patch']
    message_count = 4.0
    messages = ['399066', '399178', '399184', '399903']
    nosy_count = 3.0
    nosy_names = ['Mark.Shannon', 'Antony.Lee', 'Dennis Sweeney']
    pr_nums = ['27782']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue44850'
    versions = ['Python 3.11']

    Linked PRs

    @anntzer
    Copy link
    Mannequin Author

    anntzer mannequin commented Aug 6, 2021

    Currently, methodcaller is not faster than a plain lambda:

    In [1]: class T:
       ...:     a = 1
       ...:     def f(self): pass
       ...:     
    
    In [2]: from operator import *
    
    In [3]: %%timeit t = T(); mc = methodcaller("f")
       ...: mc(t)
       ...: 
       ...: 
    83.1 ns ± 0.862 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
    
    In [4]: %%timeit t = T(); mc = lambda x: x.f()
       ...: mc(t)
       ...: 
       ...: 
    81.4 ns ± 0.0508 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
    

    (on some machines, I find that it is even slower).

    Compare with attrgetter, which *is* faster:

    In [5]: %%timeit t = T(); ag = attrgetter("a")
       ...: ag(t)
       ...: 
       ...: 
    33.7 ns ± 0.0407 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
    
    In [6]: %%timeit t = T(); ag = lambda x: x.a
       ...: ag(t)
       ...: 
       ...: 
    50.1 ns ± 0.057 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
    

    Given that the operator module explicitly advertises itself as being "efficient"/"fast", it seems reasonable to try to optimize methodcaller. Looking at its C implementation, methodcaller currently uses PyObject_GetAttr followed by PyObject_Call; I wonder whether this can be optimized using a LOAD_METHOD-style approach to avoid the construction of the bound method (when applicable)?

    @anntzer anntzer mannequin added stdlib Python modules in the Lib dir labels Aug 6, 2021
    @sweeneyde
    Copy link
    Member

    Using _PyObject_GetMethod similarly to the way that LOAD_METHOD/CALL_METHOD does seems like a reasonable idea to me -- do you want to make a pull request? It would also be nice to see some microbenchmarks for the change once it's ready.

    @sweeneyde sweeneyde added 3.11 only security fixes performance Performance or resource usage labels Aug 7, 2021
    @sweeneyde
    Copy link
    Member

    For what it's worth, in my benchmarks on 3.11, methodcaller was already a bit faster than lambda:

    #################### Builtin calls ####################

    PS > .\python.bat -m pyperf timeit -s "from operator import methodcaller as mc" -s "reverse_it = mc('reverse')" -s "arr = []" "reverse_it(arr)"
    Running Release|x64 interpreter...
    .....................
    Mean +- std dev: 84.3 ns +- 1.9 ns

    PS >.\python.bat -m pyperf timeit -s "reverse_it = lambda x: x.reverse()" -s "arr = []" "reverse_it(arr)"
    Running Release|x64 interpreter...
    .....................
    Mean +- std dev: 95.5 ns +- 2.9 ns

    #################### Python calls ####################

    PS > .\python.bat -m pyperf timeit -s "from operator import methodcaller as mc" -s "reverse_it = mc('reverse')" -s "class A: reverse = lambda self: None" -s "arr=A()" "reverse_it(arr)"
    Running Release|x64 interpreter...
    .....................
    Mean +- std dev: 140 ns +- 4 ns
    PS > .\python.bat -m pyperf timeit -s "reverse_it = lambda x: x.reverse()" -s "class A: reverse = lambda self: None" -s "arr=A()" "reverse_it(arr)"
    Running Release|x64 interpreter...
    .....................
    Mean +- std dev: 159 ns +- 4 ns

    @markshannon
    Copy link
    Member

    If the problem is that methodcaller is not faster than a lambda, then why not use a lambda?

    Lambdas are just Python functions and calling them will get faster.

    We aren't going to spend time optimizing calls to methodcaller, so you might as well use a lambda.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added 3.12 bugs and security fixes and removed 3.11 only security fixes labels Sep 7, 2022
    @hugovk
    Copy link
    Member

    hugovk commented Dec 11, 2023

    The PR has been merged, I think we can close this now. Feel free to re-open if still needed.

    @hugovk hugovk closed this as completed Dec 11, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.12 bugs and security fixes performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants