Skip to content

bpo-36616: optimize handling of thread state in function call code #12839

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

Closed
wants to merge 1 commit into from

Conversation

jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Apr 15, 2019

Thanks to @markshannon for the idea.

This does various micro-optimizations for function calls in the bytecode interpreter to give a small increase in performance.

https://bugs.python.org/issue36616

@jdemeyer jdemeyer changed the title Optimize handling of thread state in function call code bpo-36616: optimize handling of thread state in function call code Apr 15, 2019
Copy link
Contributor

@scoder scoder left a comment

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;
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

See also my PR #12934.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants