Skip to content

gh-126868: Add freelist for compact int objects #126865

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

Merged
markshannon merged 25 commits into from
Dec 13, 2024
+102 −55
Merged

gh-126868: Add freelist for compact int objects #126865

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0713034
Add freelist of compact int objects
eendebakpt Nov 15, 2024
be58ade
fix build
eendebakpt Nov 15, 2024
3f50b54
cleanup freelist at exit
eendebakpt Nov 15, 2024
fa97302
fix memory leak; align with float implementation
eendebakpt Nov 16, 2024
d72486f
remove stale comment
eendebakpt Nov 16, 2024
e07c218
remove unused function
eendebakpt Nov 16, 2024
328e0c1
avoid some problematic decrefs
eendebakpt Nov 16, 2024
e1dc2b3
jit build
eendebakpt Nov 16, 2024
9df776b
📜🤖 Added by blurb_it.
blurb-it[bot] Nov 16, 2024
6b73046
Merge branch 'main' into int_freelist
eendebakpt Nov 16, 2024
db8247e
Apply review suggestions
Fidget-Spinner Nov 20, 2024
d1e4aa2
Fixup
Fidget-Spinner Nov 20, 2024
644e85a
review comments part 1
eendebakpt Nov 21, 2024
9f86b6e
review comments part 2
eendebakpt Nov 21, 2024
88274d6
Merge branch 'main' into int_freelist
eendebakpt Nov 21, 2024
1e548bd
review suggestion
eendebakpt Nov 23, 2024
13bc3e9
Merge branch 'main' into int_freelist
eendebakpt Dec 2, 2024
60220c0
add small int check to _PyLong_ExactDealloc
eendebakpt Dec 2, 2024
593d621
add COMPARE_OP_INT
eendebakpt Dec 2, 2024
efde111
review comment
eendebakpt Dec 7, 2024
928e912
Merge branch 'main' into int_freelist
eendebakpt Dec 7, 2024
437c24c
regenerate
eendebakpt Dec 7, 2024
b034948
Update Misc/NEWS.d/next/Core_and_Builtins/2024-11-16-22-37-46.gh-issu…
eendebakpt Dec 7, 2024
19f64f6
Merge branch 'main' into int_freelist
eendebakpt Dec 12, 2024
14681c1
Merge branch 'main' into int_freelist
eendebakpt Dec 12, 2024
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
Prev Previous commit
Next Next commit
Apply review suggestions
  • Loading branch information
@Fidget-Spinner
Fidget-Spinner committed Nov 20, 2024
commit db8247e0aa20aa33a17af6b342cc138117302ab4
18 changes: 10 additions & 8 deletions Objects/longobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static inline void
_Py_DECREF_INT(PyLongObject *op)
{
assert(PyLong_CheckExact(op));
_Py_DECREF_SPECIALIZED((PyObject *)op, (destructor)PyObject_Free); // needs to be converted to freelist?
_Py_DECREF_SPECIALIZED((PyObject *)op, (destructor) _PyLong_ExactDealloc);
Copy link
Copy Markdown
Member

@picnixz picnixz Nov 21, 2024

Choose a reason for hiding this comment

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

Suggested change
_Py_DECREF_SPECIALIZED((PyObject *)op, (destructor) _PyLong_ExactDealloc);
_Py_DECREF_SPECIALIZED((PyObject *)op, (destructor)_PyLong_ExactDealloc);

I'm also not sure whether you need the cast. If you need the cast, I'd suggest changing the signature of the drstructor itself.

}

static inline int
Expand Down Expand Up @@ -222,7 +222,9 @@ _PyLong_FromMedium(sdigit x)
assert(!IS_SMALL_INT(x));
assert(is_medium_int(x));

PyLongObject *v = _Py_FREELIST_POP(PyLongObject, ints);
// The small int cache is incompatible with _Py_NewReference which is called
// by _Py_FREELIST_POP.
PyLongObject *v = (PyLongObject *)_Py_FREELIST_POP_MEM(ints);
Copy link
Copy Markdown
Member

@markshannon markshannon Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you go back to _Py_FREELIST_POP and avoid reinitializing the object on line 237. We don't need to re-assign the ob_type field in objects taken from the freelist. Something like:

    PyLongObject *v = (PyLongObject *)_Py_FREELIST_POP(ints);
    if (v == NULL) {
        v = PyObject_Malloc(sizeof(PyLongObject));
        if (v == NULL) {
            PyErr_NoMemory();
            return NULL;
        }
        _PyObject_Init((PyObject*)v, &PyLong_Type);
    }
    digit abs_x = x < 0 ? -x : x;
    _PyLong_SetSignAndDigitCount(v, x<0?-1:1, 1);
    v->long_value.ob_digit[0] = abs_x;
    return (PyObject*)v;

if (v == NULL) {
v = PyObject_Malloc(sizeof(PyLongObject));
if (v == NULL) {
Expand Down Expand Up @@ -3618,13 +3620,13 @@ long_richcompare(PyObject *self, PyObject *other, int op)
void
_PyLong_ExactDealloc(PyObject *self)
{
assert(PyLong_CheckExact(self));

if (_PyLong_IsCompact((PyLongObject *)self)) {
_Py_FREELIST_FREE(ints, self, PyObject_Free);
return;
if (PyLong_CheckExact(self)) {
if (_PyLong_IsCompact((PyLongObject *)self)) {
_Py_FREELIST_FREE(ints, self, PyObject_Free);
return;
}
}
PyObject_Free(self);
Py_TYPE(self)->tp_free(self);
}

static void
Expand Down
6 changes: 3 additions & 3 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ dummy_func(
STAT_INC(BINARY_OP, hit);
PyObject *res_o = _PyLong_Multiply((PyLongObject *)left_o, (PyLongObject *)right_o);
PyStackRef_CLOSE_SPECIALIZED(right, (destructor)_PyLong_ExactDealloc);
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)PyObject_Free); // needs to be converted to freelist
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)_PyLong_ExactDealloc);
INPUTS_DEAD();
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand All @@ -529,7 +529,7 @@ dummy_func(
STAT_INC(BINARY_OP, hit);
PyObject *res_o = _PyLong_Add((PyLongObject *)left_o, (PyLongObject *)right_o);
PyStackRef_CLOSE_SPECIALIZED(right, (destructor)_PyLong_ExactDealloc);
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)PyObject_Free); // needs to be converted to freelist
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)_PyLong_ExactDealloc);
INPUTS_DEAD();
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand All @@ -542,7 +542,7 @@ dummy_func(
STAT_INC(BINARY_OP, hit);
PyObject *res_o = _PyLong_Subtract((PyLongObject *)left_o, (PyLongObject *)right_o);
PyStackRef_CLOSE_SPECIALIZED(right, (destructor)_PyLong_ExactDealloc);
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)PyObject_Free); // needs to be converted to freelist
PyStackRef_CLOSE_SPECIALIZED(left, (destructor)_PyLong_ExactDealloc);
INPUTS_DEAD();
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
Expand Down
6 changes: 3 additions & 3 deletions Python/executor_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Python/generated_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.