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.

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

Simplify PyStackRef_FromPyObjectSteal #127022

Open
colesbury opened this issue Nov 19, 2024 · 1 comment
Open

Simplify PyStackRef_FromPyObjectSteal #127022

colesbury opened this issue Nov 19, 2024 · 1 comment
Labels
performance Performance or resource usage topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 19, 2024

Feature or enhancement

Currently, PyStackRef_FromPyObjectSteal checks if the object is immortal in order to set the Py_TAG_DEFERRED bit.

static inline _PyStackRef
_PyStackRef_FromPyObjectSteal(PyObject *obj)
{
assert(obj != NULL);
// Make sure we don't take an already tagged value.
assert(((uintptr_t)obj & Py_TAG_BITS) == 0);
unsigned int tag = _Py_IsImmortal(obj) ? (Py_TAG_DEFERRED) : Py_TAG_PTR;
return ((_PyStackRef){.bits = ((uintptr_t)(obj)) | tag});
}
# define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj))

This check isn't necessary and has a performance cost that's not made up for by the slightly faster PyStackRef_CLOSE() calls or PyStackRef_Is() checks.

We should simplify PyStackRef_FromPyObjectSteal so that it creates _PyStackRef directly from the PyObject * without setting any tag bits.

Linked PRs

@colesbury colesbury added type-feature A feature request or enhancement performance Performance or resource usage topic-free-threading labels Nov 19, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Nov 19, 2024
This gets rid of the immortal check in `PyStackRef_FromPyObjectSteal()`.
Overall, this improves performance about 2% in the free threading
build.

This also renames `PyStackRef_Is()` to `PyStackRef_IsExactly()` because
the macro requires that the tag bits of the arguments match, which is
only true in certain special cases.
colesbury added a commit to colesbury/cpython that referenced this issue Nov 19, 2024
This gets rid of the immortal check in `PyStackRef_FromPyObjectSteal()`.
Overall, this improves performance about 2% in the free threading
build.

This also renames `PyStackRef_Is()` to `PyStackRef_IsExactly()` because
the macro requires that the tag bits of the arguments match, which is
only true in certain special cases.
@colesbury
Copy link
Contributor Author

I think this will also allow us to simplify or remove _PyEvalFramePushAndInit_UnTagged.

colesbury added a commit that referenced this issue Nov 22, 2024
This gets rid of the immortal check in `PyStackRef_FromPyObjectSteal()`.
Overall, this improves performance about 2% in the free threading
build.

This also renames `PyStackRef_Is()` to `PyStackRef_IsExactly()` because
the macro requires that the tag bits of the arguments match, which is
only true in certain special cases.
colesbury added a commit to colesbury/cpython that referenced this issue Nov 22, 2024
The interpreter now handles `_PyStackRef`s pointing to immortal objects
without the deferred bit set, so `_PyEvalFramePushAndInit_UnTagged` is
no longer necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant