Skip to content

bpo-34303: Micro-optimizations in functools.reduce() #8598

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
Jun 1, 2019

Conversation

sir-sigurd
Copy link
Contributor

@sir-sigurd sir-sigurd commented Aug 1, 2018

PyTuple_SetItem(args, 1, op2);
if ((result = PyEval_CallObject(func, args)) == NULL)
Py_XSETREF(PyTuple_GET_ITEM(args, 0), result);
Py_XSETREF(PyTuple_GET_ITEM(args, 1), op2);
Copy link
Contributor

@eric-wieser eric-wieser Aug 3, 2018

Choose a reason for hiding this comment

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

Does PyTuple_GET_ITEM guarantee its result is an lvalue? That's not in the public API anywhere.

PySequence_Fast_ITEMS(args)[0] would be conforming to the public API, and I think just as fast

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it, PySequence_Fast_ITEMS() seems to be slower as it checks for type.

@sir-sigurd sir-sigurd force-pushed the functools-reduce-performance branch from 4a0bca9 to f491abf Compare February 18, 2019 11:56
@methane methane changed the title bpo-34303: Micro-optimizations in functools.reduce(). bpo-34303: Micro-optimizations in functools.reduce() Feb 18, 2019
@csabella
Copy link
Contributor

@methane, was there any blocker on merging this? Thanks!

@methane
Copy link
Member

methane commented May 31, 2019

@methane, was there any blocker on merging this? Thanks!

@rhettinger would you review this?

@rhettinger rhettinger merged commit e5f6207 into python:master Jun 1, 2019
@sir-sigurd sir-sigurd deleted the functools-reduce-performance branch June 10, 2019 15:00
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants