Skip to content

bpo-37207: Use PEP 590 vectorcall to speed up tuple() #18936

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

Merged
merged 2 commits into from
Mar 13, 2020

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Mar 11, 2020

Master

./python.exe -m pyperf timeit "tuple()"
.....................
Mean +- std dev: 262 ns +- 9 ns

./python.exe -m pyperf timeit "tuple((1, 2, 3, 4, 5))"
.....................
Mean +- std dev: 361 ns +- 15 ns
./python.exe -m pyperf timeit "tuple([1, 2, 3, 4, 5])"
.....................
Mean +- std dev: 927 ns +- 29 ns

PEP-590

./python.exe -m pyperf timeit "tuple()"
.....................
Mean +- std dev: 176 ns +- 11 ns

./python.exe -m pyperf timeit "tuple((1, 2, 3, 4, 5))"
.....................
Mean +- std dev: 203 ns +- 13 ns

./python.exe -m pyperf timeit "tuple([1, 2, 3, 4, 5])"
.....................
Mean +- std dev: 714 ns +- 35 ns

https://bugs.python.org/issue37207

Master

./python.exe -m pyperf timeit "tuple((1, 2, 3, 4, 5))"
.....................
Mean +- std dev: 361 ns +- 15 ns

PEP-590

./python.exe -m pyperf timeit "tuple((1, 2, 3, 4, 5))"
.....................
Mean +- std dev: 203 ns +- 13 ns
@corona10
Copy link
Member Author

@markshannon @vstinner
Please take a look :)

@corona10 corona10 added the performance Performance or resource usage label Mar 11, 2020
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

size_t nargsf, PyObject *kwnames)
{
if (kwnames && PyTuple_GET_SIZE(kwnames) != 0) {
PyErr_Format(PyExc_TypeError, "tuple() takes no keyword arguments");
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add a function similar to _PyArg_NoKeywords() but accept a kwnames tuple. But it should be done in a separated PR.

return NULL;
}
Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
if (nargs > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

I would be nice to have an helper function to raise the exception for use: function taking nargsf and calling PyVectorcall_NARGS() for us. Again, it should be done in a separated PR.

What does Argument Clinic use for that? Does it duplicate the code?

I'm also worried but the Python binary will contains tons of ""xxx takes no keyword arguments" duplicated strings, where only xxx changes. _PyArg_NoKeywords() builds a string from the function name and so doesn't have the issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with your all concerns :)
I will submit the PR to deal with!

@vstinner vstinner merged commit 9ee88cd into python:master Mar 13, 2020
@corona10 corona10 deleted the bpo-37207-tuple branch March 29, 2020 00:41
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.

4 participants