GH-125174: Mark objects as statically allocated. by markshannon · Pull Request #127797 · python/cpython
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM. I've left a couple of minor comments. This will be a nice improvement.
Out of curiosity, how disruptive is adding a new field to the object structure?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This appears to be causing lots of compiler warnings on Windows. Here's what the "Files changed tab looks like on this PR:
These warnings are now showing up on the "files changed" tab on every PR
| #if SIZEOF_VOID_P > 4 | ||
| /* If an object has been freed, it will have a negative full refcnt | ||
| * If it has not it been freed, will have a very large refcnt */ | ||
| if (op->ob_refcnt_full <= 0 || op->ob_refcnt > (UINT32_MAX - (1<<20))) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently UINT32_MAX is not defined in all builds, so this breaks a buildbot.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use UINT32_MAX all over the place, so it must be defined. Maybe we're missing a #include somewhere.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's a C++ build that failing, which might explain it
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request
* 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