-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
GH-140052: Add PyTuple_MakeSingle and PyTuple_MakePair #140132
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
base: main
Are you sure you want to change the base?
GH-140052: Add PyTuple_MakeSingle and PyTuple_MakePair #140132
Conversation
Misc/NEWS.d/next/Core_and_Builtins/2025-10-15-01-30-28.gh-issue-140052.08spgX.rst
Outdated
Show resolved
Hide resolved
This reverts commit ebb70c9.
Co-authored-by: Victor Stinner <[email protected]>
|
@vstinner I have made requested changes. Please take a look. |
|
Done, please take a look. |
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
|
Microbenchmarks: Ubuntu 24.04, gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0, same cpu, built with lto enabled t - PyTuple_New + PyTuple_SetItem Microbenchmarks - sergey-miryanov@07f7c6a run scripts
|
|
In the microbenchmarks it seems odd that |
|
Microbenchmark results from Windows machine (Windows 11, i5-11600K @ 3.90GHz) Results for Tuple(EmptyTuple) and Tuple(EmptyTuple, EmptyTuple) - tuple will be tracked (EmptyTuple - is a special case - we don't allocate it) Notes:
Linux benchmarks will be a bit later. Benchmarks here - https://github.com/sergey-miryanov/cpython/tree/140052-pytuple-make-pair-bench |
|
I don't know how to read your benchmark. What are the "a", "p", "s", etc. columns? |
|
Oh, I suppose that letters are the same from previous benchmark: #140132 (comment) |
|
Yes, they are the same (except
|
|
Microbenchmark results from Linux (Ubuntu 24.04, gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0, i5-11600K @ 3.90GHz) Results for Tuple(EmptyTuple) and Tuple(EmptyTuple, EmptyTuple): Notes:
Legend:
|
|
@eendebakpt I have updated microbenchmarks. Could you please take a look? Are they fair enough now? |
|
@vstinner This is ready for review. Could you please take a look? |
| # because we only check type for gc support can't untrack tuple of | ||
| # immutable tuples, see maybe_tracked | ||
| self.assertTrue(gc.is_tracked(make_single((1, 2)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it'll be better not to check this, since we can make another decision in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a small disagreement here.
I think we should explicitly fix the current behavior in the tests.
@efimov-mikhail does not want to fix the behavior that may change in the near future because doing so would require changes to the tests.
We need another opinion on this.
The benchmarks seem fine, although I still find the results surprising (why is But I am +1 on the PR, even if we don't go looking into tiny performance details: the methods |
|
I created capi-workgroup/decisions#84 decision issue for the C API Working Group. |
As requested by @vstinner I have added a separate PR with two functions:
PyTuple_MakeSingleandPyTuple_MakePair.📚 Documentation preview 📚: https://cpython-previews--140132.org.readthedocs.build/