bpo-36616: optimize handling of thread state in function call code by jdemeyer · Pull Request #12839 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

jdemeyer

@jdemeyer jdemeyer changed the titleOptimize handling of thread state in function call code bpo-36616: optimize handling of thread state in function call code

Apr 15, 2019

@jdemeyer

Thanks to Mark Shannon for the idea

scoder

Choose a reason for hiding this comment

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

I'd say, why not, but would also like to see at least some timings.

PyObject **sp, *res;
sp = stack_pointer;
res = call_function(&sp, oparg, NULL);
stack_pointer = sp;

Choose a reason for hiding this comment

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

These stack pointer assignments look a bit like someone left them for documentation purposes, trying to make it clear what is allowed/supposed to happen with the pointer that is passed down.
I''m not saying that it's wrong to remove these assignments, just that Chesterton's Fence might apply.

Choose a reason for hiding this comment

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

Those strange redirections were added because somebody wanted to use

register PyObject **stack_pointer;

and it's illegal to take the address of a register variable. But the register keyword is obsolete, compilers are now much more clever than they were before. Also, call_function() should be inlined and then the pointer isn't even a real pointer.

@jdemeyer

I did some more extensive benchmarks today and I'm no longer convinced that this actually improves anything. In any case, the gain would be very close to the measurement error and not significant.

Parts of this could still make sense as a "clean up" patch, for example the strange assignment of stack_pointer around call_function(). But I don't want to fight that battle, so I'll just close this.

@vstinner