◐ Shell
reader mode source ↗
Skip to content

GH-125174: Mark objects as statically allocated.#127797

Merged
markshannon merged 11 commits into
python:mainfrom
faster-cpython:refcnt-with-flags
Dec 11, 2024
Merged

GH-125174: Mark objects as statically allocated.#127797
markshannon merged 11 commits into
python:mainfrom
faster-cpython:refcnt-with-flags

Conversation

@markshannon

@markshannon markshannon commented Dec 10, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

This PR is a follow up to #125251 which made checking for immortal objects more robust.

This adds the ability to check whether objects are statically allocated or not, which should assist finalization and safety checks.

@bedevere-app bedevere-app Bot mentioned this pull request Dec 10, 2024
Closed
ericsnowcurrently
ericsnowcurrently reviewed Dec 10, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Mostly LGTM. I've left a couple of minor comments. This will be a nice improvement.

@ZeroIntensity

ZeroIntensity commented Dec 11, 2024

Copy link
Copy Markdown
Member

Out of curiosity, how disruptive is adding a new field to the object structure?

@markshannon

markshannon commented Dec 11, 2024

Copy link
Copy Markdown
Member Author

Out of curiosity, how disruptive is adding a new field to the object structure?

It doesn't change the size of the PyObject struct or the offsets of ob_refcnt and ob_type.
Changing any of those would be very disruptive.

ericsnowcurrently
ericsnowcurrently approved these changes Dec 11, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM

@markshannon markshannon merged commit bc262de into python:main Dec 11, 2024
@AlexWaygood

AlexWaygood commented Dec 11, 2024

Copy link
Copy Markdown
Member

This appears to be causing lots of compiler warnings on Windows. Here's what the "Files changed tab looks like on this PR:
image

These warnings are now showing up on the "files changed" tab on every PR

@markshannon markshannon mentioned this pull request Dec 12, 2024
Merged
@mpage mpage mentioned this pull request Dec 12, 2024
Closed
@ZeroIntensity ZeroIntensity mentioned this pull request Dec 18, 2024
Closed
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@markshannon @srinivasreddy
pythonGH-125174: Mark objects as statically allocated. (python#127797) …
fb66075 
* Set a bit in the unused part of the refcount on 64 bit machines and the free-threaded build.

* Use the top of the refcount range on 32 bit machines
@markshannon markshannon deleted the refcnt-with-flags branch January 10, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@markshannon @ZeroIntensity @AlexWaygood @encukou @ericsnowcurrently