Skip to content

bpo-37233: optimize case totalargs == 0 in method_vectorcall - #14550

Merged
methane merged 1 commit into
python:masterfrom
jdemeyer:method_vectorcall
Jul 3, 2019
Merged

bpo-37233: optimize case totalargs == 0 in method_vectorcall#14550
methane merged 1 commit into
python:masterfrom
jdemeyer:method_vectorcall

Conversation

@jdemeyer

@jdemeyerjdemeyer commented Jul 2, 2019•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Because of bpo-37138, we need to check the case totalargs == 0 anyway. So it doesn't hurt to move that check earlier and add a fast path for the common case of calling a method without arguments.

CC @methane

https://bugs.python.org/issue37233

@gpshead
gpshead requested a review from methaneJuly 2, 2019 22:09
@methane

methane commented Jul 3, 2019

Copy link
Copy Markdown
Member

I'm not sure this is really optimization.
Isn't (nargsf & PY_VECTORCALL_ARGUMENTS_OFFSET) true on major case?

We need if (totalargs) { at C level. But it doesn't mean this check is needed in assembly level.
Compilers may optimize out this check if it is safe.

Would you show microbenchmark result for this?

@jdemeyer

jdemeyer commented Jul 3, 2019

Copy link
Copy Markdown
ContributorAuthor

Compilers may optimize out this check if it is safe.

I checked and they don't optimize away this check. So my original reasoning is still correct: if this check has to be done anyway, we should put it as early as possible.

@methane
methane merged commit 53c2143 into python:masterJul 3, 2019
@miss-islington

miss-islington commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

Thanks @jdemeyer for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

Sorry, @jdemeyer and @methane, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 53c214344038341ce86fcf7efa12dc33be9d5b45 3.8

@jdemeyer
jdemeyer deleted the method_vectorcall branch July 3, 2019 12:01
@jdemeyer

jdemeyer commented Jul 3, 2019

Copy link
Copy Markdown
ContributorAuthor

If you want to backport this, you should also backport #13974

@methane

methane commented Jul 3, 2019

Copy link
Copy Markdown
Member

OK, I want to keep 3.8 and 3.9 same to ease future backporting.
#13974 seems safe enough to backport too.

@miss-islington

miss-islington commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

Thanks @jdemeyer for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 3, 2019
@jdemeyer@miss-islington
bpo-37233: optimize method_vectorcall in case of totalargs == 0 (pyth…
03cf5f2
…onGH-14550)
(cherry picked from commit 53c2143)
Co-authored-by: Jeroen Demeyer <J.Demeyer@UGent.be>
@bedevere-bot

bedevere-bot commented Jul 3, 2019

Copy link
Copy Markdown

GH-14574 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Jul 3, 2019
@miss-islington@jdemeyer
bpo-37233: optimize method_vectorcall in case of totalargs == 0 (GH-1…
1099e34
…4550)
(cherry picked from commit 53c2143)
Co-authored-by: Jeroen Demeyer <J.Demeyer@UGent.be>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@jdemeyer@lisroach
bpo-37233: optimize method_vectorcall in case of totalargs == 0 (pyth…
88a4ecf
…onGH-14550)
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@jdemeyer@DinoV
bpo-37233: optimize method_vectorcall in case of totalargs == 0 (pyth…
95f5d8e
…onGH-14550)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methanemethaneAwaiting requested review from methane

Assignees

@methanemethane

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jdemeyer@methane@miss-islington@bedevere-bot@gpshead@the-knights-who-say-ni