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.

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

gh-111968: Use per-thread freelists for float in free-threading #113886

Merged
merged 4 commits into from
Jan 10, 2024
Merged

gh-111968: Use per-thread freelists for float in free-threading #113886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
100072e
gh-111968: Use per-thread freelists for float in free-threading
corona10 Jan 10, 2024
2bebcdc
nit
corona10 Jan 10, 2024
47e6fc6
Update Include/internal/pycore_freelist.h
corona10 Jan 10, 2024
b05f268
Address code review
corona10 Jan 10, 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
21 changes: 2 additions & 19 deletions Include/internal/pycore_floatobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif


#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_unicodeobject.h" // _PyUnicodeWriter

/* runtime lifecycle */

extern void _PyFloat_InitState(PyInterpreterState *);
extern PyStatus _PyFloat_InitTypes(PyInterpreterState *);
extern void _PyFloat_Fini(PyInterpreterState *);
extern void _PyFloat_Fini(_PyFreeListState *);
extern void _PyFloat_FiniType(PyInterpreterState *);


Expand All @@ -33,24 +33,7 @@ struct _Py_float_runtime_state {
};


#ifndef WITH_FREELISTS
// without freelists
# define PyFloat_MAXFREELIST 0
#endif

#ifndef PyFloat_MAXFREELIST
# define PyFloat_MAXFREELIST 100
#endif

struct _Py_float_state {
#if PyFloat_MAXFREELIST > 0
/* Special free list
free_list is a singly-linked list of available PyFloatObjects,
linked via abuse of their ob_type members. */
int numfree;
PyFloatObject *free_list;
#endif
};

void _PyFloat_ExactDealloc(PyObject *op);

Expand Down
26 changes: 18 additions & 8 deletions Include/internal/pycore_freelist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,34 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#ifndef WITH_FREELISTS
// without freelists
# define PyList_MAXFREELIST 0
#endif

/* Empty list reuse scheme to save calls to malloc and free */
#ifndef PyList_MAXFREELIST
#ifdef WITH_FREELISTS
// with freelists
# define PyList_MAXFREELIST 80
# define PyFloat_MAXFREELIST 100
#else
# define PyList_MAXFREELIST 0
# define PyFloat_MAXFREELIST 0
#endif

struct _Py_list_state {
#if PyList_MAXFREELIST > 0
#ifdef WITH_FREELISTS
PyListObject *free_list[PyList_MAXFREELIST];
int numfree;
#endif
};

struct _Py_float_state {
#ifdef WITH_FREELISTS
/* Special free list
free_list is a singly-linked list of available PyFloatObjects,
linked via abuse of their ob_type members. */
int numfree;
PyFloatObject *free_list;
#endif
};

typedef struct _Py_freelist_state {
struct _Py_float_state float_state;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the same naming scheme? The list freelist state member is named list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because float is reserved name?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we decide to change the naming schme, list should be changed. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the naming of a struct and its members is an important part of API design. Having a consistent scheme makes it easier to read and review code. I would suggest use a naming scheme for the freelist struct and its members. Up to you :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree, I will create a seperate PR :)

struct _Py_list_state list;
} _PyFreeListState;

Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_gc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ extern PyObject *_PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs);
extern void _PyGC_ClearAllFreeLists(PyInterpreterState *interp);
extern void _Py_ClearFreeLists(_PyFreeListState *state, int is_finalization);
extern void _PyTuple_ClearFreeList(PyInterpreterState *interp);
extern void _PyFloat_ClearFreeList(PyInterpreterState *interp);
extern void _PyFloat_ClearFreeList(_PyFreeListState *state, int is_finalization);
extern void _PyList_ClearFreeList(_PyFreeListState *state, int is_finalization);
extern void _PyDict_ClearFreeList(PyInterpreterState *interp);
extern void _PyAsyncGen_ClearFreeLists(PyInterpreterState *interp);
Expand Down
1 change: 0 additions & 1 deletion Include/internal/pycore_interp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ struct _is {
#endif
struct _py_object_state object_state;
struct _Py_unicode_state unicode;
struct _Py_float_state float_state;
struct _Py_long_state long_state;
struct _dtoa_state dtoa;
struct _py_func_state func_state;
Expand Down
47 changes: 20 additions & 27 deletions Objects/floatobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,13 @@ class float "PyObject *" "&PyFloat_Type"

#include "clinic/floatobject.c.h"

#ifndef PyFloat_MAXFREELIST
# define PyFloat_MAXFREELIST 100
#endif


#if PyFloat_MAXFREELIST > 0
#ifdef WITH_FREELISTS
static struct _Py_float_state *
get_float_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
return &interp->float_state;
_PyFreeListState *state = _PyFreeListState_GET();
assert(state != NULL);
return &state->float_state;
}
#endif

Expand Down Expand Up @@ -132,7 +128,7 @@ PyObject *
PyFloat_FromDouble(double fval)
{
PyFloatObject *op;
#if PyFloat_MAXFREELIST > 0
#ifdef WITH_FREELISTS
struct _Py_float_state *state = get_float_state();
op = state->free_list;
if (op != NULL) {
Expand Down Expand Up @@ -252,13 +248,9 @@ _PyFloat_ExactDealloc(PyObject *obj)
{
assert(PyFloat_CheckExact(obj));
PyFloatObject *op = (PyFloatObject *)obj;
#if PyFloat_MAXFREELIST > 0
#ifdef WITH_FREELISTS
struct _Py_float_state *state = get_float_state();
#ifdef Py_DEBUG
// float_dealloc() must not be called after _PyFloat_Fini()
assert(state->numfree != -1);
#endif
if (state->numfree >= PyFloat_MAXFREELIST) {
if (state->numfree >= PyFloat_MAXFREELIST || state->numfree < 0) {
PyObject_Free(op);
return;
}
Expand All @@ -275,7 +267,7 @@ static void
float_dealloc(PyObject *op)
{
assert(PyFloat_Check(op));
#if PyFloat_MAXFREELIST > 0
#ifdef WITH_FREELISTS
if (PyFloat_CheckExact(op)) {
_PyFloat_ExactDealloc(op);
}
Expand Down Expand Up @@ -2002,29 +1994,30 @@ _PyFloat_InitTypes(PyInterpreterState *interp)
}

void
_PyFloat_ClearFreeList(PyInterpreterState *interp)
_PyFloat_ClearFreeList(_PyFreeListState *freelist_state, int is_finalization)
{
#if PyFloat_MAXFREELIST > 0
struct _Py_float_state *state = &interp->float_state;
#ifdef WITH_FREELISTS
struct _Py_float_state *state = &freelist_state->float_state;
PyFloatObject *f = state->free_list;
while (f != NULL) {
PyFloatObject *next = (PyFloatObject*) Py_TYPE(f);
PyObject_Free(f);
f = next;
}
state->free_list = NULL;
state->numfree = 0;
if (is_finalization) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, we only set num_free to -1 during finalization in debug builds. With this change, we set it in non-debug builds too. I think that's fine (and simpler), but we should better handle the -1 case in PyFloat_ExactDealloc in case assertions are not enabled.

I'd suggest adding changing the condition in PyFloat_ExactDealloc to something like if (state->num_free >= PyFloat_MAXFREELIST || state->num_free < 0) so that num_free=-1 disables the free-list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

state->numfree = -1;
}
else {
state->numfree = 0;
}
#endif
}

void
_PyFloat_Fini(PyInterpreterState *interp)
_PyFloat_Fini(_PyFreeListState *state)
{
_PyFloat_ClearFreeList(interp);
#if defined(Py_DEBUG) && PyFloat_MAXFREELIST > 0
struct _Py_float_state *state = &interp->float_state;
state->numfree = -1;
#endif
_PyFloat_ClearFreeList(state, 1);
}

void
Expand All @@ -2037,7 +2030,7 @@ _PyFloat_FiniType(PyInterpreterState *interp)
void
_PyFloat_DebugMallocStats(FILE *out)
{
#if PyFloat_MAXFREELIST > 0
#ifdef WITH_FREELISTS
struct _Py_float_state *state = get_float_state();
_PyDebugAllocatorStats(out,
"free PyFloatObject",
Expand Down
1 change: 0 additions & 1 deletion Python/gc_free_threading.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ void
_PyGC_ClearAllFreeLists(PyInterpreterState *interp)
{
_PyTuple_ClearFreeList(interp);
_PyFloat_ClearFreeList(interp);
Copy link
Member Author

Choose a reason for hiding this comment

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

Integrated with _Py_ClearFreeLists

_PyDict_ClearFreeList(interp);
_PyAsyncGen_ClearFreeLists(interp);
_PyContext_ClearFreeList(interp);
Expand Down
1 change: 0 additions & 1 deletion Python/gc_gil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ void
_PyGC_ClearAllFreeLists(PyInterpreterState *interp)
{
_PyTuple_ClearFreeList(interp);
_PyFloat_ClearFreeList(interp);
Copy link
Member Author

Choose a reason for hiding this comment

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

Integrated with _Py_ClearFreeLists

_PyDict_ClearFreeList(interp);
_PyAsyncGen_ClearFreeLists(interp);
_PyContext_ClearFreeList(interp);
Expand Down
2 changes: 1 addition & 1 deletion Python/pylifecycle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1757,10 +1757,10 @@ finalize_interp_types(PyInterpreterState *interp)
_PySlice_Fini(interp);

_PyUnicode_Fini(interp);
_PyFloat_Fini(interp);

_PyFreeListState *state = _PyFreeListState_GET();
_PyList_Fini(state);
_PyFloat_Fini(state);

#ifdef Py_DEBUG
_PyStaticObjects_CheckRefcnt(interp);
Expand Down
1 change: 1 addition & 0 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,7 @@ clear_datastack(PyThreadState *tstate)
void
_Py_ClearFreeLists(_PyFreeListState *state, int is_finalization)
{
_PyFloat_ClearFreeList(state, is_finalization);
_PyList_ClearFreeList(state, is_finalization);
}

Expand Down
Loading