Skip to content

Add Py_TuplePack2 and Py_TuplePack1 #118222

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
eendebakpt opened this issue Apr 24, 2024 · 3 comments
Closed

Add Py_TuplePack2 and Py_TuplePack1 #118222

eendebakpt opened this issue Apr 24, 2024 · 3 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@eendebakpt
Copy link
Contributor

eendebakpt commented Apr 24, 2024

Feature or enhancement

Proposal:

In the cpython codebase PyTuple_Pack is used at various places (https://github.com/search?q=repo%3Apython%2Fcpython+PyTuple_Pack&type=code). The execution is not very fast as the implementation uses va_arg internally. For the 1- and 2 argument case we can improve performance by providing a direct implementation.

Using PyTuple_Pack2 (example implementation main...eendebakpt:cpython:putuple_pack2) performance of components like pairwise can be improved. See https://discuss.python.org/t/nwise-itertools/51718/22

  • Can we add Py_TuplePack1 and Py_TuplePack2 to the cpython interface? If so, should it be in the public or private API.
  • If we add Py_TuplePack2 should we use it at all places applicable, or only the few performance critical ones?
  • An alternative for Py_TuplePack1 that already exists is the internal _PyTuple_FromArray with second argument 1.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

@serhiy-storchaka
Copy link
Member

How large is the difference between PyTuple_Pack(), _PyTuple_FromArray(), PyTuple_Pack1() and PyTuple_Pack2()?

I afraid that it is a preliminary optimization. We need new API if:

  • It allows to express what was not possible with the old API. It is obvious.
  • It allows to avoid the overhead of more general API for common cases.
  • It is much more convenient in common cases. Usually it means less lines of code.

PyTuple_Pack() is already a convenient API in comparison with PyTuple_New() + Py_INCREF() + PyTuple_SET_ITEM(). And it is fast enough, faster than more general Py_BuildValue(), perhaps even faster than PyTuple_New() + Py_IncRef() + PyTuple_SetItem().

@eendebakpt
Copy link
Contributor Author

@serhiy-storchaka Thanks for the response. I agree the need for a new API seems slim. The only reason would be performance.

Here are some results: with the benchmark script from https://discuss.python.org/t/nwise-itertools/51718/17 we can compare the performance of the cpython pairwise (implemented in C) and a simple recipe (python only). With current main the python recipe is faster than the cpython C version.

Main:

 11.1 ± 0.1 μs  consume(recipe(iterable))
 13.1 ± 0.2 μs  consume(pairwise(iterable))

Python: 3.13.0a6+ (heads/putuple_pack2:8f25cc9920, Apr 30 2024, 11:26:49) [MSC v.1939 64 bit (AMD64)]

With PyTuple_Pack2 (implementation main...eendebakpt:cpython:putuple_pack2) the two versions are almost as fast:

 11.1 ± 0.1 μs  consume(recipe(iterable))
 11.5 ± 0.0 μs  consume(pairwise(iterable))

Python: 3.13.0a6+ (heads/putuple_pack2-dirty:f2431b38e7, Apr 30 2024, 11:23:04) [MSC v.1939 64 bit (AMD64)]

So using PyTuple_Pack2 gives a performance improvement of 10-15%.

I will keep this issue open a few more days to see if there are different opinions or more use cases where the performance difference is important. If not I will close the issue and (perhaps) make a PR to update just the pairwise implementation with a direct PyTuple_New() + Py_INCREF() + PyTuple_SET_ITEM()

@eendebakpt
Copy link
Contributor Author

Looking through the cpython code there are only few places where the usage of PyTuple_Pack is performance critical. One had been adressed in #118219, another is addressed in #118703. Closing this issue

@terryjreedy terryjreedy closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants