Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expand pyatomic coverage #108337

Closed
Tracked by #108219
colesbury opened this issue Aug 22, 2023 · 2 comments
Closed
Tracked by #108219

Expand pyatomic coverage #108337

colesbury opened this issue Aug 22, 2023 · 2 comments
Labels
3.13 new features, bugs and security fixes topic-C-API type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Aug 22, 2023 •
edited by bedevere-bot

Feature or enhancement

Implementing PEP 703 requires use of atomic operations on more data types than provided by pycore_atomic.h. Additionally, pycore_atomic.h is only usable from Py_BUILD_CORE modules; it can't be used in public headers. PEP 703 will require atomic operations in object.h for Py_INCREF/DECREF, for example.

I propose adding a new header to wrap platform/C11 provided atomic operations:

  • The functions will be exposed in the public C-API but prefixed with an underscore to indicate they are private. They need to be exposed because they will be used by other public inline functions.
  • The functions operate on plain data types (i.e. int instead of _Py_atomic_int). This allows use of atomic operations on existing data types where we don't want to change the types of contained fields (i.e, the fields of PyTypeObject.) and avoids some issues regarding mixing C and C++.
  • Use of GCC built-in atomics when available. These are available in GCC 4.8+. They do not require passing -std=gnu11 or -std=c11 to the compiler, which avoids breaking third-party extensions.
  • The memory order is part of the function name instead of an argument. This means there are more function definitions (e.g., both _Py_atomic_load_int and _Py_atomic_load_int_relaxed) but keeps the implementation of each function simpler, particularly for MSVC. This reduces boiler-plate code when stepping through debug builds.

Linked PRs
@colesbury colesbury added type-feature A feature request or enhancement topic-C-API 3.13 new features, bugs and security fixes labels Aug 22, 2023
@colesbury colesbury mentioned this issue Aug 22, 2023
Open
colesbury added a commit to colesbury/cpython that referenced this issue Aug 22, 2023
@colesbury
pythongh-108337: Add pyatomic.h header
23a2c0b 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
colesbury added a commit to colesbury/cpython that referenced this issue Aug 22, 2023
@colesbury
pythongh-108337: Add pyatomic.h header
2bc82a6 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
@bedevere-bot bedevere-bot mentioned this issue Aug 22, 2023
Closed
vstinner pushed a commit to vstinner/cpython that referenced this issue Aug 30, 2023
@colesbury @vstinner
pythongh-108337: Add pyatomic.h header
19f46c1 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
@bedevere-bot bedevere-bot mentioned this issue Aug 30, 2023
Merged
vstinner pushed a commit to vstinner/cpython that referenced this issue Aug 30, 2023
@colesbury @vstinner
pythongh-108337: Add pyatomic.h header
d8cc195 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
vstinner pushed a commit to vstinner/cpython that referenced this issue Aug 31, 2023
@colesbury @vstinner
pythongh-108337: Add pyatomic.h header
d8dc286 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
vstinner pushed a commit to vstinner/cpython that referenced this issue Aug 31, 2023
@colesbury @vstinner
pythongh-108337: Add pyatomic.h header
261a8a6 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.
vstinner added a commit that referenced this issue Aug 31, 2023
@vstinner @colesbury
gh-108337: Add pyatomic.h header (#108701)
2bd960b 
This adds a new header that provides atomic operations on common data
types. The intention is that this will be exposed through Python.h,
although that is not the case yet. The only immediate use is in
the test file.

Co-authored-by: Sam Gross <colesbury@gmail.com>
@colesbury colesbury mentioned this issue Sep 14, 2023
Merged
@colesbury colesbury mentioned this issue Sep 21, 2023
Closed
@scoder
Copy link
Contributor

scoder commented Apr 25, 2024

The leading underscore marks these as "private". However, with the advent of a GIL-free Python, these will become increasingly useful for third-party code. Why consider them private?

@colesbury
Copy link
Contributor Author

colesbury commented Apr 25, 2024 •
edited

There is ongoing discussion about whether they should be made public, but currently no consensus. Some of the reasons to keep them private:

  • When MSVC finally adds support C11 atomics, we may want to use C11 atomics directly or at least simplify the API.
  • Many extensions have better options, such as Rust atomics or C11 or C++11 atomics
  • Some maintainers believe that it is out of scope for CPython: basically that CPython shouldn't be providing cross-platform wrappers for OS or compiler APIs, but limited to focusing on APIs that interact with CPython.

If we maintained a similar header-only atomics library outside this repo, would that be useful for Cython? Basically, something like https://github.com/python/pythoncapi-compat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.13 new features, bugs and security fixes topic-C-API type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vstinner @scoder @colesbury