Skip to content

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Dec 23, 2024

@corona10
Copy link
MemberAuthor

@colesbury I am not sure what you intended.
To maintain the original size, I squeeze the state field from 32 bits to 16 bits and reserve 16 bits for the interned field.

@corona10
Copy link
MemberAuthor

3: Interned, Immortal, and Static
This categorization allows the runtime to determine the right
cleanup mechanism at runtime shutdown. */
uint8_tinterned;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this remain in the state struct. It's okay for a struct to contain both non-bitfield and bitfield members:

  • It avoids a potential unnecessary breakage from moving the field
  • Keeping it in state will make it easier to keep state 32-bits due to alignment.

@colesbury
Copy link
Contributor

I think this should have a NEWS entry given that we're changing the size of a semi-public field.

@corona10corona10 changed the title gh-128137: Split out interned field from state[WIP] gh-128137: Split out interned field from stateDec 23, 2024
@corona10corona10 changed the title [WIP] gh-128137: Split out interned field from stategh-128137: Split out interned field from stateDec 23, 2024
@corona10corona10 changed the title gh-128137: Split out interned field from stategh-128137: Update PyASCIIObject layout to handle interned field with the atomic operation.Dec 23, 2024
@corona10corona10 changed the title gh-128137: Update PyASCIIObject layout to handle interned field with the atomic operation.gh-128137: Update PyASCIIObject to handle interned field with the atomic operationDec 23, 2024
@corona10
Copy link
MemberAuthor

corona10 commented Dec 23, 2024

Since we touch the semi-public field, I am not sure about backporting this PR.
Let's just leave 3.13t as buggy status because it is just the experimental build.

@corona10corona10 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 2, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 91907aa 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 2, 2025
@corona10
Copy link
MemberAuthor

corona10 commented Jan 2, 2025

I think that it would be fine

  • uint16_t: interned field = 16bits (Increase it to 16bits is little bit painful, but it would be nothing for modern CPUs)
  • unsigned short: kind + compact + ascii + statically_allocated + padding = 16bits (for 32bits and 64bits system both)
  • In total: 32 bits

@corona10
Copy link
MemberAuthor

In Linux

  • sizeof PyASCIIObject is 40bytes
  • sizeof PyASCIIObject.state is 4 bytes.

So no regression will be.

Copy link
Contributor

@colesburycolesbury left a comment

Choose a reason for hiding this comment

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

This LGTM. @methane, would you please take a look at this as well?

@corona10corona10 merged commit ae23a01 into python:mainJan 5, 2025
118 checks passed
@corona10corona10 deleted the gh-128137 branch January 5, 2025 09:17
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 6, 2025
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Jan 21, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Jan 23, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Feb 6, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Feb 13, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 4, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 10, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 19, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 20, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 24, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Mar 31, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Apr 10, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
ngoldbaum pushed a commit to clin1234/pyo3 that referenced this pull request Apr 10, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
ngoldbaum pushed a commit to clin1234/pyo3 that referenced this pull request Apr 11, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
clin1234 added a commit to clin1234/pyo3 that referenced this pull request Apr 17, 2025
Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14.
github-merge-queuebot pushed a commit to PyO3/pyo3 that referenced this pull request Apr 26, 2025
* Add news item * Move news file * Fix version limit check in noxfile.py * Bump Python version for testing debug builds * 3.14 is available from GH's setup-python action * Bump maximum supported CPython version in pyo3-ffi * Rework PyASCIIObject and PyUnicodeObject to be compatible with 3.14 Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14. * Run `cargo fmt --all` * Actually add Py_3_14 as a legitimate macro When `rustc` is invoked, the macro is included with the `--check-cfg` flag, but not with the `--cfg` flag. This caused errors about duplicate definitions to spew out when building with stable Rust toolchains. * Revert "Actually add Py_3_14 as a legitimate macro" This reverts commit 5da57af. * Fix version macro placement for 3.14-specific getters and setters * Import 'c_ushort' only if compiling against CPython 3.14 or later * Add wrapper functions for the statically_allocated field * Remove unused libc::c_ushort * Add (hopefully) final version-specific macros * Port 3.14-specific 64-bit code of Py_INCREF * Don't expose PyDictObject.ma_version_tag when building against 3.14 or later * fix ffi-check on the GIL-enabled ABI * fix older pythons * fix ffi-check on older pythons * WIP: update for 3.14t * fix ffi-check on the free-threaded build * fix clippy * fix clippy on older python versions * fix cargo check on the MSRV * fix ffi-check on 3.13t * fix CI which is using 3.13.1 * fix copy/paste error in noxfile * update ffi bindings for the latest changes in 3.14 * update layout of refcnt field on gil-enabled build * delete unused HangThread struct * fix ffi-check on GIL-enabled build * Revert "delete unused HangThread struct" This reverts commit 3dd439d. * config-out HangThread * fix 3.13 ffi-check * fix debug python build error * fix graalpy build * Ignore DeprecationWarnings from the pytest_asyncio module in tests * Add abi3-py314 * fix free-threading issue in `test_coroutine` (#5069) * Introspection: add function signatures (#5025) * Introspection: add function signatures No annotations or explicit default values yet Fixes an issue related to object identifiers path * Better default value * Refine arguments struct * Introduce VariableLengthArgument * Adds pyfunctions tests * Adds some serialization tests * respond to david's code review * add comment and fix test failure * fix check-feature-powerset * fix clippy * fix wasip1 clippy * fix 32 bit python 3.14 bug * mark test-py step continue-on-error for dev python builds * use github issue URL * run ffi-check before running tests * fix ffi-check for 3.14.0a7 --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> Co-authored-by: David Hewitt <mail@davidhewitt.dev> Co-authored-by: Thomas Tanon <thomas.pellissier-tanon@helsing.ai>
newcomertv pushed a commit to newcomertv/pyo3 that referenced this pull request Apr 28, 2025
* Add news item * Move news file * Fix version limit check in noxfile.py * Bump Python version for testing debug builds * 3.14 is available from GH's setup-python action * Bump maximum supported CPython version in pyo3-ffi * Rework PyASCIIObject and PyUnicodeObject to be compatible with 3.14 Due to python/cpython#128196, data types within `PyASCIIObject.state` have changed, resulting in test failures when building against 3.14. * Run `cargo fmt --all` * Actually add Py_3_14 as a legitimate macro When `rustc` is invoked, the macro is included with the `--check-cfg` flag, but not with the `--cfg` flag. This caused errors about duplicate definitions to spew out when building with stable Rust toolchains. * Revert "Actually add Py_3_14 as a legitimate macro" This reverts commit 5da57af. * Fix version macro placement for 3.14-specific getters and setters * Import 'c_ushort' only if compiling against CPython 3.14 or later * Add wrapper functions for the statically_allocated field * Remove unused libc::c_ushort * Add (hopefully) final version-specific macros * Port 3.14-specific 64-bit code of Py_INCREF * Don't expose PyDictObject.ma_version_tag when building against 3.14 or later * fix ffi-check on the GIL-enabled ABI * fix older pythons * fix ffi-check on older pythons * WIP: update for 3.14t * fix ffi-check on the free-threaded build * fix clippy * fix clippy on older python versions * fix cargo check on the MSRV * fix ffi-check on 3.13t * fix CI which is using 3.13.1 * fix copy/paste error in noxfile * update ffi bindings for the latest changes in 3.14 * update layout of refcnt field on gil-enabled build * delete unused HangThread struct * fix ffi-check on GIL-enabled build * Revert "delete unused HangThread struct" This reverts commit 3dd439d. * config-out HangThread * fix 3.13 ffi-check * fix debug python build error * fix graalpy build * Ignore DeprecationWarnings from the pytest_asyncio module in tests * Add abi3-py314 * fix free-threading issue in `test_coroutine` (PyO3#5069) * Introspection: add function signatures (PyO3#5025) * Introspection: add function signatures No annotations or explicit default values yet Fixes an issue related to object identifiers path * Better default value * Refine arguments struct * Introduce VariableLengthArgument * Adds pyfunctions tests * Adds some serialization tests * respond to david's code review * add comment and fix test failure * fix check-feature-powerset * fix clippy * fix wasip1 clippy * fix 32 bit python 3.14 bug * mark test-py step continue-on-error for dev python builds * use github issue URL * run ffi-check before running tests * fix ffi-check for 3.14.0a7 --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> Co-authored-by: David Hewitt <mail@davidhewitt.dev> Co-authored-by: Thomas Tanon <thomas.pellissier-tanon@helsing.ai>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@corona10@colesbury@vstinner@bedevere-bot@methane@kumaraditya303