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

bpo-46944: use FASTCALL calling convention in generator.throw #31723

Merged
merged 5 commits into from Mar 11, 2022

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Mar 7, 2022

@kumaraditya303 kumaraditya303 changed the title use FASTCALL calling convention in generator.throw bpo-46944: use FASTCALL calling convention in generator.throw Mar 7, 2022
@kumaraditya303 kumaraditya303 marked this pull request as ready for review March 7, 2022 11:57
@corona10
Copy link
Member

corona10 commented Mar 7, 2022

@kumaraditya303 Can you provide the benchmark?

@AlexWaygood AlexWaygood added the performance Performance or resource usage label Mar 7, 2022
@kumaraditya303
Copy link
Contributor Author

kumaraditya303 commented Mar 7, 2022

@kumaraditya303 Can you provide the benchmark?

@corona10
Micro Benchmark (I used pyperf):

import pyperf

runner = pyperf.Runner()
runner.timeit(name="bench throw",
              stmt="g.throw(TypeError)",
              setup = """
def gen():
    while True:
        try:
            yield
        except Exception:
            pass
g = gen()
g.send(None)
""")

Result:

Mean +- std dev: [base] 264 ns +- 12 ns -> [patch] 218 ns +- 12 ns: 1.21x faster

@markshannon
Copy link
Member

OOI, why did you choose generator.throw?
There are many builtin functions that use METH_VARARGS and could be ported to METH_FASTCALL.

I don't want to discourage you, I'm just curious.
I'd like to port as many builtin functions as possible to the faster calling conventions.

@kumaraditya303
Copy link
Contributor Author

OOI, why did you choose generator.throw?
There are many builtin functions that use METH_VARARGS and could be ported to METH_FASTCALL.

I was reading through the generator implementation and noticed that throw does not uses the fastcall calling convention so I created a PR to make it fast.

I'll find more function which are worth porting over to fastcall. Thanks for your suggestion.

@arhadthedev
Copy link
Member

arhadthedev commented Mar 7, 2022

If a vararg method looks like throw(typ[,val[,tb]]), can it be remolded as throw(typ, val=None, tb=None)?

Edit: or rather throw(typ, val=None, tb=None, /)

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@markshannon
Copy link
Member

Looks good

@markshannon markshannon merged commit 304197b into python:main Mar 11, 2022
@kumaraditya303 kumaraditya303 deleted the generator branch March 11, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants