Skip to content

gh-129695: Optimize _PyFloat_FromDouble_ConsumeInputs() #129697

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+43 −18
Open

gh-129695: Optimize _PyFloat_FromDouble_ConsumeInputs() #129697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b856732
Optimize ``_PyFloat_FromDouble_ConsumeInputs()``
nascheme Jan 29, 2025
970949e
Add NEWS.
nascheme Feb 5, 2025
ecf15ed
Fix bug, _Py_DECREF_DecRefTotal().
nascheme Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions Include/internal/pycore_object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,17 +261,53 @@ _Py_DECREF_NO_DEALLOC(PyObject *op)
}

#else
// TODO: implement Py_DECREF specializations for Py_GIL_DISABLED build

static inline void
_Py_DECREF_SPECIALIZED(PyObject *op, const destructor destruct)
{
Py_DECREF(op);
if (_Py_IsOwnedByCurrentThread(op) &&
_Py_atomic_load_ssize_relaxed(&op->ob_ref_shared) == 0) {
uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
if (local == _Py_IMMORTAL_REFCNT_LOCAL) {
_Py_DECREF_IMMORTAL_STAT_INC();
return;
}
_Py_DECREF_STAT_INC();
#ifdef Py_REF_DEBUG
_Py_DECREF_DecRefTotal();
#endif
local--;
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local);
if (local == 0) {
destruct(op);
}
}
else {
Py_DECREF(op);
}
}

static inline void
_Py_DECREF_NO_DEALLOC(PyObject *op)
{
Py_DECREF(op);
if (_Py_IsOwnedByCurrentThread(op) &&
_Py_atomic_load_ssize_relaxed(&op->ob_ref_shared) == 0) {
uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local);
if (local == _Py_IMMORTAL_REFCNT_LOCAL) {
_Py_DECREF_IMMORTAL_STAT_INC();
return;
}
_Py_DECREF_STAT_INC();
#ifdef Py_REF_DEBUG
_Py_DECREF_DecRefTotal();
#endif
local--;
assert(local > 0);
_Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local);
}
else {
Py_DECREF(op);
}
}

static inline int
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-02-05-11-11-40.gh-issue-129695.qt2Jsz.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add an optimized version of the ``_PyFloat_FromDouble_ConsumeInputs``
function for the free-threaded build.
17 changes: 2 additions & 15 deletions Objects/floatobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,27 +134,16 @@ PyFloat_FromDouble(double fval)
return (PyObject *) op;
}

#ifdef Py_GIL_DISABLED

PyObject *_PyFloat_FromDouble_ConsumeInputs(_PyStackRef left, _PyStackRef right, double value)
{
PyStackRef_CLOSE(left);
PyStackRef_CLOSE(right);
return PyFloat_FromDouble(value);
}

#else // Py_GIL_DISABLED

PyObject *_PyFloat_FromDouble_ConsumeInputs(_PyStackRef left, _PyStackRef right, double value)
Copy link
Contributor

@colesbury colesbury Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should inline this into the call sites, even at the cost of repeating code. I don't think the stackpointer manipulations will be correct otherwise. (cc @markshannon)

We should also use PyStackRef_CLOSE_SPECIALIZED instead of _Py_DECREF_SPECIALIZED. The same applies to _Py_DECREF_NO_DEALLOC, but I think that stackref variant will need to be added.

{
PyObject *left_o = PyStackRef_AsPyObjectSteal(left);
PyObject *right_o = PyStackRef_AsPyObjectSteal(right);
if (Py_REFCNT(left_o) == 1) {
if (_PyObject_IsUniquelyReferenced(left_o)) {
((PyFloatObject *)left_o)->ob_fval = value;
_Py_DECREF_SPECIALIZED(right_o, _PyFloat_ExactDealloc);
return left_o;
}
else if (Py_REFCNT(right_o) == 1) {
else if (_PyObject_IsUniquelyReferenced(right_o)) {
((PyFloatObject *)right_o)->ob_fval = value;
_Py_DECREF_NO_DEALLOC(left_o);
return right_o;
Expand All @@ -167,8 +156,6 @@ PyObject *_PyFloat_FromDouble_ConsumeInputs(_PyStackRef left, _PyStackRef right,
}
}

#endif // Py_GIL_DISABLED

static PyObject *
float_from_string_inner(const char *s, Py_ssize_t len, void *obj)
{
Expand Down
Loading