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

gh-111968: Introduce _PyFreeListState and _PyFreeListState_GET API #113584

Merged
merged 54 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
cd4863c
gh-111968: Introduce _Py_freelist_state and _PyFreeListState_GET API
corona10 Dec 30, 2023
333873d
nit
corona10 Dec 30, 2023
8b7e261
Remove comment
corona10 Dec 30, 2023
dd0afa2
Update _PyList_Fini
corona10 Dec 30, 2023
f0d55de
nit refactor
corona10 Dec 31, 2023
5e8e3b4
fix
corona10 Dec 31, 2023
7aa956a
Update finalize step
corona10 Dec 31, 2023
908ef13
pep7
corona10 Dec 31, 2023
0096383
Update
corona10 Jan 2, 2024
bc8dd4a
Update
corona10 Jan 2, 2024
40bad6b
fix
corona10 Jan 2, 2024
1b0f370
fix
corona10 Jan 2, 2024
3d6042e
fix
corona10 Jan 2, 2024
09403db
fix
corona10 Jan 2, 2024
c05491e
Update
corona10 Jan 2, 2024
d5f9559
fix
corona10 Jan 2, 2024
3dd3c3f
update
corona10 Jan 2, 2024
0d8ea53
nit
corona10 Jan 2, 2024
e9d8138
Address code review
corona10 Jan 2, 2024
07e0536
Update Python/pylifecycle.c
corona10 Jan 2, 2024
6c8ba67
Address code review
corona10 Jan 2, 2024
8a80970
Address code review
corona10 Jan 2, 2024
6d2f2d4
nit
corona10 Jan 2, 2024
887bacf
Use _Py_FinalizeFreeLists
corona10 Jan 2, 2024
e65952f
nit
corona10 Jan 2, 2024
25a6714
fix
corona10 Jan 2, 2024
3fd9baa
fix
corona10 Jan 2, 2024
e175b61
fix
corona10 Jan 2, 2024
295a292
nit
corona10 Jan 2, 2024
5836c4b
nit
corona10 Jan 2, 2024
7cb261f
nit
corona10 Jan 2, 2024
01bbc7a
Revert naming of clear_all_freelists
corona10 Jan 2, 2024
b08f65f
Use _Py_ClearFreeLists as possible
corona10 Jan 2, 2024
65cedee
nit
corona10 Jan 2, 2024
9a40708
Fix finalize code
corona10 Jan 2, 2024
af86a20
nit
corona10 Jan 2, 2024
4e84130
Merge remote-tracking branch 'upstream/main' into gh-111968
corona10 Jan 5, 2024
7ffa64c
Split implementation
corona10 Jan 5, 2024
d6a2feb
Add files
corona10 Jan 5, 2024
458aadb
nit
corona10 Jan 5, 2024
b105bda
nit
corona10 Jan 5, 2024
18e9216
fix
corona10 Jan 5, 2024
5bb8d3f
nit
corona10 Jan 5, 2024
2811a12
fix
corona10 Jan 6, 2024
a5f494d
fix compiler warn
corona10 Jan 6, 2024
837ae60
nit
corona10 Jan 6, 2024
08c8613
fix
corona10 Jan 6, 2024
b5eb472
Adjust comment
corona10 Jan 6, 2024
0d8dc3d
2 tabs, not 8 spaces
erlend-aasland Jan 6, 2024
fe859e1
Rename to _PyGC_ClearAllFreeLists
corona10 Jan 9, 2024
c284061
Pass is_finalization to _PyList_ClearFreeList
corona10 Jan 9, 2024
a8e39b3
fix
corona10 Jan 9, 2024
99ac375
fix
corona10 Jan 9, 2024
0c331c7
fix
corona10 Jan 9, 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
35 changes: 35 additions & 0 deletions Include/internal/pycore_freelist.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#ifndef Py_INTERNAL_FREELIST_H
Copy link
Member Author

Choose a reason for hiding this comment

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

All freelist related domains will be moved here.

#define Py_INTERNAL_FREELIST_H
#ifdef __cplusplus
extern "C" {
#endif

#ifndef Py_BUILD_CORE
# 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
# define PyList_MAXFREELIST 80
#endif
Comment on lines +11 to +19
Copy link
Member Author

@corona10 corona10 Jan 2, 2024

Choose a reason for hiding this comment

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

Can be improved better way to only depend on WITH_FREELISTS but let's improve it with the separate PRs.
It is just copied and pasted code from

#ifndef WITH_FREELISTS
// without freelists
# define PyList_MAXFREELIST 0
#endif
/* Empty list reuse scheme to save calls to malloc and free */
#ifndef PyList_MAXFREELIST
# define PyList_MAXFREELIST 80
#endif
struct _Py_list_state {
#if PyList_MAXFREELIST > 0
PyListObject *free_list[PyList_MAXFREELIST];
int numfree;
#endif
};


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

typedef struct _Py_freelist_state {
struct _Py_list_state list;
} _PyFreeListState;

#ifdef __cplusplus
}
#endif
#endif /* !Py_INTERNAL_FREELIST_H */
6 changes: 5 additions & 1 deletion Include/internal/pycore_gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h" // _PyFreeListState

/* GC information is stored BEFORE the object structure. */
typedef struct {
// Pointer to next object in the list.
Expand Down Expand Up @@ -238,14 +240,16 @@ extern PyObject *_PyGC_GetObjects(PyInterpreterState *interp, Py_ssize_t generat
extern PyObject *_PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs);

// Functions to clear types free lists
extern void _Py_ClearFreeLists(_PyFreeListState *state);
Copy link
Member

Choose a reason for hiding this comment

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

Hummm, I find this asymmetry a bit odd. Wouldn't be better to keep all functions receiving PyInterpreterState *interp and then fetching the freelist state from there instead?

That way the code of the caller will be also cleaner and doesn't need to deal with where the freelist is stored.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the free-threaded builds, we want to clear the free-lists for a specific thread (not all threads). Passing interp doesn't provide enough information. In some cases the thread we are clearing is not even the active thread, such as when we call PyThreadState_Clear() on dead threads after fork.

extern void _PyTuple_ClearFreeList(PyInterpreterState *interp);
extern void _PyFloat_ClearFreeList(PyInterpreterState *interp);
extern void _PyList_ClearFreeList(PyInterpreterState *interp);
extern void _PyList_ClearFreeList(_PyFreeListState *state);
extern void _PyDict_ClearFreeList(PyInterpreterState *interp);
extern void _PyAsyncGen_ClearFreeLists(PyInterpreterState *interp);
extern void _PyContext_ClearFreeList(PyInterpreterState *interp);
extern void _Py_ScheduleGC(PyInterpreterState *interp);
extern void _Py_RunGC(PyThreadState *tstate);
extern void _PyGC_Clear_FreeList(PyInterpreterState *interp);

#ifdef __cplusplus
}
Expand Down
4 changes: 3 additions & 1 deletion Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ struct _is {
// One bit is set for each non-NULL entry in code_watchers
uint8_t active_code_watchers;

#if !defined(Py_GIL_DISABLED)
struct _Py_freelist_state freelist_state;
#endif
struct _py_object_state object_state;
struct _Py_unicode_state unicode;
struct _Py_float_state float_state;
Expand All @@ -190,7 +193,6 @@ struct _is {
PySliceObject *slice_cache;

struct _Py_tuple_state tuple;
struct _Py_list_state list;
struct _Py_dict_state dict_state;
struct _Py_async_gen_state async_gen;
struct _Py_context_state context;
Expand Down
22 changes: 2 additions & 20 deletions Include/internal/pycore_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,17 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h" // _PyFreeListState

extern PyObject* _PyList_Extend(PyListObject *, PyObject *);
extern void _PyList_DebugMallocStats(FILE *out);


/* runtime lifecycle */

extern void _PyList_Fini(PyInterpreterState *);
extern void _PyList_Fini(_PyFreeListState *);


/* other API */

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

/* Empty list reuse scheme to save calls to malloc and free */
#ifndef PyList_MAXFREELIST
# define PyList_MAXFREELIST 80
#endif

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

#define _PyList_ITEMS(op) _Py_RVALUE(_PyList_CAST(op)->ob_item)

extern int
Expand Down
16 changes: 16 additions & 0 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_runtime.h" // _PyRuntime
#include "pycore_tstate.h" // _PyThreadStateImpl


// Values for PyThreadState.state. A thread must be in the "attached" state
Expand Down Expand Up @@ -239,6 +241,20 @@ PyAPI_FUNC(const PyConfig*) _Py_GetConfig(void);
// See also PyInterpreterState_Get() and _PyInterpreterState_GET().
extern PyInterpreterState* _PyGILState_GetInterpreterStateUnsafe(void);

static inline _PyFreeListState* _PyFreeListState_GET(void)
{
PyThreadState *tstate = _PyThreadState_GET();
#ifdef Py_DEBUG
_Py_EnsureTstateNotNULL(tstate);
#endif

#ifdef Py_GIL_DISABLED
return &((_PyThreadStateImpl*)tstate)->freelist_state;
#else
return &tstate->interp->freelist_state;
#endif
}

#ifdef __cplusplus
}
#endif
Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_tstate.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h" // struct _Py_freelist_state
#include "pycore_mimalloc.h" // struct _mimalloc_thread_state


Expand All @@ -20,6 +21,7 @@ typedef struct _PyThreadStateImpl {

#ifdef Py_GIL_DISABLED
struct _mimalloc_thread_state mimalloc;
struct _Py_freelist_state freelist_state;
#endif

} _PyThreadStateImpl;
Expand Down
3 changes: 3 additions & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ PYTHON_OBJS= \
Python/frozenmain.o \
Python/future.o \
Python/gc.o \
Python/gc_free_threading.o \
Python/gc_gil.o \
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
Python/getargs.o \
Python/getcompiler.o \
Python/getcopyright.o \
Expand Down Expand Up @@ -1828,6 +1830,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_floatobject.h \
$(srcdir)/Include/internal/pycore_format.h \
$(srcdir)/Include/internal/pycore_frame.h \
$(srcdir)/Include/internal/pycore_freelist.h \
$(srcdir)/Include/internal/pycore_function.h \
$(srcdir)/Include/internal/pycore_genobject.h \
$(srcdir)/Include/internal/pycore_getopt.h \
Expand Down
16 changes: 8 additions & 8 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ _Py_DECLARE_STR(list_err, "list index out of range");
static struct _Py_list_state *
get_list_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
return &interp->list;
_PyFreeListState *state = _PyFreeListState_GET();
assert(state != NULL);
return &state->list;
}
#endif

Expand Down Expand Up @@ -120,10 +121,10 @@ list_preallocate_exact(PyListObject *self, Py_ssize_t size)
}

void
_PyList_ClearFreeList(PyInterpreterState *interp)
_PyList_ClearFreeList(_PyFreeListState *freelist_state)
{
#if PyList_MAXFREELIST > 0
struct _Py_list_state *state = &interp->list;
struct _Py_list_state *state = &freelist_state->list;
while (state->numfree) {
PyListObject *op = state->free_list[--state->numfree];
assert(PyList_CheckExact(op));
Expand All @@ -133,12 +134,11 @@ _PyList_ClearFreeList(PyInterpreterState *interp)
}

void
_PyList_Fini(PyInterpreterState *interp)
_PyList_Fini(_PyFreeListState *state)
{
_PyList_ClearFreeList(interp);
_PyList_ClearFreeList(state);
#if defined(Py_DEBUG) && PyList_MAXFREELIST > 0
struct _Py_list_state *state = &interp->list;
state->numfree = -1;
state->list.numfree = -1;
#endif
}

Expand Down
2 changes: 2 additions & 0 deletions PCbuild/_freeze_module.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@
<ClCompile Include="..\Python\frame.c" />
<ClCompile Include="..\Python\future.c" />
<ClCompile Include="..\Python\gc.c" />
<ClCompile Include="..\Python\gc_gil.c" />
<ClCompile Include="..\Python\gc_free_threading.c" />
<ClCompile Include="..\Python\getargs.c" />
<ClCompile Include="..\Python\getcompiler.c" />
<ClCompile Include="..\Python\getcopyright.c" />
Expand Down
6 changes: 6 additions & 0 deletions PCbuild/_freeze_module.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@
<ClCompile Include="..\Python\gc.c">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="..\Python\gc_free_threading.c">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="..\Python\gc_gil.c">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="..\Modules\gcmodule.c">
<Filter>Source Files</Filter>
</ClCompile>
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/pythoncore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@
<ClInclude Include="..\Include\internal\pycore_floatobject.h" />
<ClInclude Include="..\Include\internal\pycore_format.h" />
<ClInclude Include="..\Include\internal\pycore_frame.h" />
<ClInclude Include="..\Include\internal\pycore_freelist.h" />
<ClInclude Include="..\Include\internal\pycore_function.h" />
<ClInclude Include="..\Include\internal\pycore_gc.h" />
<ClInclude Include="..\Include\internal\pycore_genobject.h" />
Expand Down Expand Up @@ -567,6 +568,8 @@
</ClCompile>
<ClCompile Include="..\Python\future.c" />
<ClCompile Include="..\Python\gc.c" />
<ClCompile Include="..\Python\gc_free_threading.c" />
<ClCompile Include="..\Python\gc_gil.c" />
<ClCompile Include="..\Python\getargs.c" />
<ClCompile Include="..\Python\getcompiler.c" />
<ClCompile Include="..\Python\getcopyright.c" />
Expand Down
12 changes: 12 additions & 0 deletions PCbuild/pythoncore.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,12 @@
<ClInclude Include="..\Include\internal\pycore_format.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_frame.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_freelist.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_function.h">
<Filter>Include\internal</Filter>
</ClInclude>
Expand Down Expand Up @@ -1283,6 +1289,12 @@
<ClCompile Include="..\Python\gc.c">
<Filter>Python</Filter>
</ClCompile>
<ClCompile Include="..\Python\gc_free_threading.c">
<Filter>Python</Filter>
</ClCompile>
<ClCompile Include="..\Python\gc_gil.c">
<Filter>Python</Filter>
</ClCompile>
<ClCompile Include="..\Python\getargs.c">
<Filter>Python</Filter>
</ClCompile>
Expand Down
17 changes: 1 addition & 16 deletions Python/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1019,21 +1019,6 @@ delete_garbage(PyThreadState *tstate, GCState *gcstate,
}
}

/* Clear all free lists
* All free lists are cleared during the collection of the highest generation.
* Allocated items in the free list may keep a pymalloc arena occupied.
* Clearing the free lists may give back memory to the OS earlier.
*/
static void
clear_freelists(PyInterpreterState *interp)
{
_PyTuple_ClearFreeList(interp);
_PyFloat_ClearFreeList(interp);
_PyList_ClearFreeList(interp);
_PyDict_ClearFreeList(interp);
_PyAsyncGen_ClearFreeLists(interp);
_PyContext_ClearFreeList(interp);
}

// Show stats for objects in each generations
static void
Expand Down Expand Up @@ -1449,7 +1434,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason)
/* Clear free list only during the collection of the highest
* generation */
if (generation == NUM_GENERATIONS-1) {
clear_freelists(tstate->interp);
_PyGC_Clear_FreeList(tstate->interp);
}

if (_PyErr_Occurred(tstate)) {
Expand Down
30 changes: 30 additions & 0 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#ifdef Py_GIL_DISABLED

#include "Python.h"
#include "pycore_pystate.h" // _PyFreeListState_GET()
#include "pycore_tstate.h" // _PyThreadStateImpl

/* Clear all free lists
* All free lists are cleared during the collection of the highest generation.
* Allocated items in the free list may keep a pymalloc arena occupied.
* Clearing the free lists may give back memory to the OS earlier.
*/
void
_PyGC_Clear_FreeList(PyInterpreterState *interp)
{
_PyTuple_ClearFreeList(interp);
_PyFloat_ClearFreeList(interp);
_PyDict_ClearFreeList(interp);
_PyAsyncGen_ClearFreeLists(interp);
_PyContext_ClearFreeList(interp);

HEAD_LOCK(&_PyRuntime);
Copy link
Member

Choose a reason for hiding this comment

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

This locks interpreters creation, but not thread creation, no? Is it possible that this races with thread creation/destruction? I am missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

This part looks okay to me. The HEAD_LOCK() protects the threads linked list (among other things). Threads acquire the HEAD_LOCK() when appending (or removing) themselves to that list.

cpython/Python/pystate.c

Lines 1370 to 1371 in ace4d7f

/* We serialize concurrent creation to protect global state. */
HEAD_LOCK(runtime);

Modification of the free-lists themselves is protected by the "stop-the-world" pause (or the GIL), which is not yet implemented.

_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)interp->threads.head;
while (tstate != NULL) {
_Py_ClearFreeLists(&tstate->freelist_state);
tstate = tstate->base.next;
}
HEAD_UNLOCK(&_PyRuntime);
}

#endif
23 changes: 23 additions & 0 deletions Python/gc_gil.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#ifndef Py_GIL_DISABLED

#include "Python.h"
#include "pycore_pystate.h" // _Py_ClearFreeLists()

/* Clear all free lists
* All free lists are cleared during the collection of the highest generation.
* Allocated items in the free list may keep a pymalloc arena occupied.
* Clearing the free lists may give back memory to the OS earlier.
*/
void
_PyGC_Clear_FreeList(PyInterpreterState *interp)
{
_PyTuple_ClearFreeList(interp);
_PyFloat_ClearFreeList(interp);
_PyDict_ClearFreeList(interp);
_PyAsyncGen_ClearFreeLists(interp);
_PyContext_ClearFreeList(interp);

_Py_ClearFreeLists(&interp->freelist_state);

Check failure on line 20 in Python/gc_gil.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build and test (x64)

'freelist_state': is not a member of '_is' [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 20 in Python/gc_gil.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build and test (x64)

'_Py_ClearFreeLists': too few arguments for call [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 20 in Python/gc_gil.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build (arm64)

'freelist_state': is not a member of '_is' [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 20 in Python/gc_gil.c

View workflow job for this annotation

GitHub Actions / Windows (free-threading) / build (arm64)

'_Py_ClearFreeLists': too few arguments for call [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 20 in Python/gc_gil.c

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threading) / build and test

‘PyInterpreterState’ {aka ‘struct _is’} has no member named ‘freelist_state’; did you mean ‘float_state’?
}

#endif
5 changes: 4 additions & 1 deletion Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1752,13 +1752,16 @@ finalize_interp_types(PyInterpreterState *interp)
_PyUnicode_ClearInterned(interp);

_PyDict_Fini(interp);
_PyList_Fini(interp);
_PyTuple_Fini(interp);

_PySlice_Fini(interp);

_PyUnicode_Fini(interp);
_PyFloat_Fini(interp);

_PyFreeListState *state = _PyFreeListState_GET();
corona10 marked this conversation as resolved.
Show resolved Hide resolved
_PyList_Fini(state);

#ifdef Py_DEBUG
_PyStaticObjects_CheckRefcnt(interp);
#endif
Expand Down
Loading
Loading