Skip to content

bpo-40428: Remove PyTuple_ClearFreeList() function #19769

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

Merged
merged 1 commit into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions Doc/whatsnew/3.9.rst
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,19 @@ Build and C API Changes
the garbage collector respectively. (Contributed by Pablo Galindo in
:issue:`40241`.)

* Remove the following functions from the C API. Call :c:func:`PyGC_Collect`
explicitly to free all free lists.
(Contributed by Victor Stinner in :issue:`40428`.)
Comment on lines +676 to +678
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 sentence "Call :c:func:PyGC_Collect explicitly to free all free lists", "free all free lists" (although technically correct) is not especially clear without the context of how the GC works. If one reads it literally, they may wonder "why would you need to explicitly free lists that are already free?".

If my own understanding is correct, I think it might make it a bit more clear if we were to instead use "immediately deallocate all free lists". It doesn't go into detail, which wouldn't make sense here, but I think it's less ambiguous. IMO, it also reads better than "free all free lists".

Copy link
Contributor

@aeros aeros Apr 29, 2020

Choose a reason for hiding this comment

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

Also, as a more minor grammar fix, the second sentence could be started with "Instead, ", as in "Instead, call :c:func:PyGC_Collect explicitly to free all free lists." . If the above isn't worthwhile though, I don't think that would be worth a PR on its own.

Copy link
Member Author

Choose a reason for hiding this comment

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

The best would be to document PyGC_Collect()... but currently, this function is not documented in Doc/c-api/ !

"why would you need to explicitly free lists that are already free?".

Is "Call PyGC_Collect() explicitly to clear all free lists." better?

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to propose a PR to enhance What's New In Python 3.9. English is not my first language ;-)

Copy link
Contributor

@aeros aeros Apr 29, 2020

Choose a reason for hiding this comment

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

@vstinner

Is "Call PyGC_Collect() explicitly to clear all free lists." better?

Yeah, that's definitely an improvement.

Feel free to propose a PR to enhance What's New In Python 3.9. English is not my first language ;-)

Sounds good, will do. I might also look into documenting PyGC_Collect(), but I'll likely require some feedback since the C-API isn't my area of expertise. Would you mind if I requested your review on it to make sure it's correct from a technical perspective?

Also, I think you have good English skills, especially for a non-native speaker! As a native speaker who's had to go through several college English courses though, I'm glad to help where I can with the documentation; similarly to how I'd ask you or others for help w/ the C-API. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I created PR #19793 to group other C API changes related to free lists. I replace "free" with "clear" there ;-)

Copy link
Contributor

@aeros aeros Apr 30, 2020

Choose a reason for hiding this comment

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

@vstinner Thanks for the update. I was about to work on it tomorrow otherwise, just finishing up my finals for the semester. :-)

I still plan on looking into documenting PyGC_Collect, but I need to more closely analyze the implementation and existing docstring first to make sure I get the details right. I'll open a PR and request a review when it's ready (if that's okay).

Copy link
Member Author

Choose a reason for hiding this comment

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

I still plan on looking into documenting PyGC_Collect, but I need to more closely analyze the implementation and existing docstring first to make sure I get the details right. I'll open a PR and request a review when it's ready (if that's okay).

Well, there is an easy way to document PyGC_Collect: it's similar to the Python gc.collect() (called without arguments), except that PyGC_Collect() does nothing if the GC is disabled.


* ``PyAsyncGen_ClearFreeLists()``
* ``PyContext_ClearFreeList()``
* ``PyDict_ClearFreeList()``
* ``PyFloat_ClearFreeList()``
* ``PyFrame_ClearFreeList()``
* ``PyList_ClearFreeList()``
* ``PySet_ClearFreeList()``
* ``PyTuple_ClearFreeList()``


Deprecated
==========
Expand Down
3 changes: 0 additions & 3 deletions Include/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ PyAPI_FUNC(int) PyContextVar_Reset(PyObject *var, PyObject *token);
PyAPI_FUNC(PyObject *) _PyContext_NewHamtForTests(void);


PyAPI_FUNC(int) PyContext_ClearFreeList(void);


#endif /* !Py_LIMITED_API */

#ifdef __cplusplus
Expand Down
2 changes: 0 additions & 2 deletions Include/cpython/dictobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ PyObject *_PyDict_Pop_KnownHash(PyObject *, PyObject *, Py_hash_t, PyObject *);
PyObject *_PyDict_FromKeys(PyObject *, PyObject *, PyObject *);
#define _PyDict_HasSplitTable(d) ((d)->ma_values != NULL)

PyAPI_FUNC(int) PyDict_ClearFreeList(void);

/* Like PyDict_Merge, but override can be 0, 1 or 2. If override is 0,
the first occurrence of a key wins, if override is 1, the last occurrence
of a key wins, if override is 2, a KeyError with conflicting key as
Expand Down
2 changes: 0 additions & 2 deletions Include/cpython/frameobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ PyAPI_FUNC(void) PyFrame_LocalsToFast(PyFrameObject *, int);
PyAPI_FUNC(int) PyFrame_FastToLocalsWithError(PyFrameObject *f);
PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *);

PyAPI_FUNC(int) PyFrame_ClearFreeList(void);

PyAPI_FUNC(void) _PyFrame_DebugMallocStats(FILE *out);

#ifdef __cplusplus
Expand Down
1 change: 0 additions & 1 deletion Include/cpython/listobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ typedef struct {
} PyListObject;

PyAPI_FUNC(PyObject *) _PyList_Extend(PyListObject *, PyObject *);
PyAPI_FUNC(int) PyList_ClearFreeList(void);
PyAPI_FUNC(void) _PyList_DebugMallocStats(FILE *out);

/* Macro, trading safety for speed */
Expand Down
3 changes: 0 additions & 3 deletions Include/floatobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ PyAPI_FUNC(double) _PyFloat_Unpack2(const unsigned char *p, int le);
PyAPI_FUNC(double) _PyFloat_Unpack4(const unsigned char *p, int le);
PyAPI_FUNC(double) _PyFloat_Unpack8(const unsigned char *p, int le);

/* free list api */
PyAPI_FUNC(int) PyFloat_ClearFreeList(void);

PyAPI_FUNC(void) _PyFloat_DebugMallocStats(FILE* out);

/* Format the object based on the format_spec, as defined in PEP 3101
Expand Down
2 changes: 0 additions & 2 deletions Include/genobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ PyAPI_FUNC(PyObject *) PyAsyncGen_New(PyFrameObject *,

PyObject *_PyAsyncGenValueWrapperNew(PyObject *);

int PyAsyncGen_ClearFreeLists(void);

#endif

#undef _PyGenObject_HEAD
Expand Down
10 changes: 10 additions & 0 deletions Include/internal/pycore_gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,16 @@ struct _gc_runtime_state {

PyAPI_FUNC(void) _PyGC_InitState(struct _gc_runtime_state *);


// Functions to clear types free lists
extern void _PyFrame_ClearFreeList(void);
extern void _PyTuple_ClearFreeList(void);
extern void _PyFloat_ClearFreeList(void);
extern void _PyList_ClearFreeList(void);
extern void _PyDict_ClearFreeList(void);
extern void _PyAsyncGen_ClearFreeLists(void);
extern void _PyContext_ClearFreeList(void);

#ifdef __cplusplus
}
#endif
Expand Down
1 change: 0 additions & 1 deletion Include/setobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ PyAPI_DATA(PyObject *) _PySet_Dummy;

PyAPI_FUNC(int) _PySet_NextEntry(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash);
PyAPI_FUNC(int) _PySet_Update(PyObject *set, PyObject *iterable);
PyAPI_FUNC(int) PySet_ClearFreeList(void);

#endif /* Section excluded by Py_LIMITED_API */

Expand Down
2 changes: 0 additions & 2 deletions Include/tupleobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ PyAPI_FUNC(int) PyTuple_SetItem(PyObject *, Py_ssize_t, PyObject *);
PyAPI_FUNC(PyObject *) PyTuple_GetSlice(PyObject *, Py_ssize_t, Py_ssize_t);
PyAPI_FUNC(PyObject *) PyTuple_Pack(Py_ssize_t, ...);

PyAPI_FUNC(int) PyTuple_ClearFreeList(void);

#ifndef Py_LIMITED_API
# define Py_CPYTHON_TUPLEOBJECT_H
# include "cpython/tupleobject.h"
Expand Down
11 changes: 11 additions & 0 deletions Misc/NEWS.d/next/C API/2020-04-28-23-17-27.bpo-40428.rmtpru.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Remove the following functions from the C API. Call :c:func:`PyGC_Collect`
explicitly to free all free lists.

* ``PyAsyncGen_ClearFreeLists()``
* ``PyContext_ClearFreeList()``
* ``PyDict_ClearFreeList()``
* ``PyFloat_ClearFreeList()``
* ``PyFrame_ClearFreeList()``
* ``PyList_ClearFreeList()``
* ``PySet_ClearFreeList()``
* ``PyTuple_ClearFreeList()``
16 changes: 7 additions & 9 deletions Modules/gcmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "pycore_object.h"
#include "pycore_pyerrors.h"
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "frameobject.h" // PyFrame_ClearFreeList
#include "pydtrace.h"
#include "pytime.h" // _PyTime_GetMonotonicClock()

Expand Down Expand Up @@ -1026,14 +1025,13 @@ delete_garbage(PyThreadState *tstate, GCState *gcstate,
static void
clear_freelists(void)
{
(void)PyFrame_ClearFreeList();
(void)PyTuple_ClearFreeList();
(void)PyFloat_ClearFreeList();
(void)PyList_ClearFreeList();
(void)PyDict_ClearFreeList();
(void)PySet_ClearFreeList();
(void)PyAsyncGen_ClearFreeLists();
(void)PyContext_ClearFreeList();
_PyFrame_ClearFreeList();
_PyTuple_ClearFreeList();
_PyFloat_ClearFreeList();
_PyList_ClearFreeList();
_PyDict_ClearFreeList();
_PyAsyncGen_ClearFreeLists();
_PyContext_ClearFreeList();
}

// Show stats for objects in each generations
Expand Down
11 changes: 4 additions & 7 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,20 +257,17 @@ static int numfreekeys = 0;

#include "clinic/dictobject.c.h"

int
PyDict_ClearFreeList(void)
void
_PyDict_ClearFreeList(void)
{
PyDictObject *op;
int ret = numfree + numfreekeys;
while (numfree) {
op = free_list[--numfree];
PyDictObject *op = free_list[--numfree];
assert(PyDict_CheckExact(op));
PyObject_GC_Del(op);
}
while (numfreekeys) {
PyObject_FREE(keys_free_list[--numfreekeys]);
}
return ret;
}

/* Print summary info about the state of the optimized allocator */
Expand All @@ -285,7 +282,7 @@ _PyDict_DebugMallocStats(FILE *out)
void
_PyDict_Fini(void)
{
PyDict_ClearFreeList();
_PyDict_ClearFreeList();
}

#define DK_SIZE(dk) ((dk)->dk_size)
Expand Down
11 changes: 4 additions & 7 deletions Objects/floatobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1998,25 +1998,22 @@ _PyFloat_Init(void)
return 1;
}

int
PyFloat_ClearFreeList(void)
void
_PyFloat_ClearFreeList(void)
{
PyFloatObject *f = free_list, *next;
int i = numfree;
while (f) {
for (; f; f = next) {
next = (PyFloatObject*) Py_TYPE(f);
PyObject_FREE(f);
f = next;
}
free_list = NULL;
numfree = 0;
return i;
}

void
_PyFloat_Fini(void)
{
(void)PyFloat_ClearFreeList();
_PyFloat_ClearFreeList();
}

/* Print summary info about the state of the optimized allocator */
Expand Down
9 changes: 3 additions & 6 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1192,25 +1192,22 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear)
}

/* Clear out the free list */
int
PyFrame_ClearFreeList(void)
void
_PyFrame_ClearFreeList(void)
{
int freelist_size = numfree;

while (free_list != NULL) {
PyFrameObject *f = free_list;
free_list = free_list->f_back;
PyObject_GC_Del(f);
--numfree;
}
assert(numfree == 0);
return freelist_size;
}

void
_PyFrame_Fini(void)
{
(void)PyFrame_ClearFreeList();
_PyFrame_ClearFreeList();
}

/* Print summary info about the state of the optimized allocator */
Expand Down
10 changes: 3 additions & 7 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1429,11 +1429,9 @@ PyAsyncGen_New(PyFrameObject *f, PyObject *name, PyObject *qualname)
}


int
PyAsyncGen_ClearFreeLists(void)
void
_PyAsyncGen_ClearFreeLists(void)
{
int ret = ag_value_freelist_free + ag_asend_freelist_free;

while (ag_value_freelist_free) {
_PyAsyncGenWrappedValue *o;
o = ag_value_freelist[--ag_value_freelist_free];
Expand All @@ -1447,14 +1445,12 @@ PyAsyncGen_ClearFreeLists(void)
assert(Py_IS_TYPE(o, &_PyAsyncGenASend_Type));
PyObject_GC_Del(o);
}

return ret;
}

void
_PyAsyncGen_Fini(void)
{
PyAsyncGen_ClearFreeLists();
_PyAsyncGen_ClearFreeLists();
}


Expand Down
11 changes: 4 additions & 7 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,23 +103,20 @@ list_preallocate_exact(PyListObject *self, Py_ssize_t size)
static PyListObject *free_list[PyList_MAXFREELIST];
static int numfree = 0;

int
PyList_ClearFreeList(void)
void
_PyList_ClearFreeList(void)
{
PyListObject *op;
int ret = numfree;
while (numfree) {
op = free_list[--numfree];
PyListObject *op = free_list[--numfree];
assert(PyList_CheckExact(op));
PyObject_GC_Del(op);
}
return ret;
}

void
_PyList_Fini(void)
{
PyList_ClearFreeList();
_PyList_ClearFreeList();
}

/* Print summary info about the state of the optimized allocator */
Expand Down
6 changes: 0 additions & 6 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2384,12 +2384,6 @@ PySet_Add(PyObject *anyset, PyObject *key)
return set_add_key((PySetObject *)anyset, key);
}

int
PySet_ClearFreeList(void)
{
return 0;
}

void
_PySet_Fini(void)
{
Expand Down
18 changes: 7 additions & 11 deletions Objects/tupleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -955,26 +955,22 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
return 0;
}

int
PyTuple_ClearFreeList(void)
void
_PyTuple_ClearFreeList(void)
{
int freelist_size = 0;
#if PyTuple_MAXSAVESIZE > 0
int i;
for (i = 1; i < PyTuple_MAXSAVESIZE; i++) {
PyTupleObject *p, *q;
p = free_list[i];
freelist_size += numfree[i];
for (Py_ssize_t i = 1; i < PyTuple_MAXSAVESIZE; i++) {
PyTupleObject *p = free_list[i];
free_list[i] = NULL;
numfree[i] = 0;
while (p) {
q = p;
PyTupleObject *q = p;
p = (PyTupleObject *)(p->ob_item[0]);
PyObject_GC_Del(q);
}
}
// the empty tuple singleton is only cleared by _PyTuple_Fini()
#endif
return freelist_size;
}

void
Expand All @@ -985,7 +981,7 @@ _PyTuple_Fini(void)
* rely on the fact that an empty tuple is a singleton. */
Py_CLEAR(free_list[0]);

(void)PyTuple_ClearFreeList();
_PyTuple_ClearFreeList();
#endif
}

Expand Down
2 changes: 0 additions & 2 deletions PC/python3.def
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ EXPORTS
PyBytes_Size=python39.PyBytes_Size
PyBytes_Type=python39.PyBytes_Type DATA
PyCFunction_Call=python39.PyCFunction_Call
PyCFunction_ClearFreeList=python39.PyCFunction_ClearFreeList
PyCFunction_GetFlags=python39.PyCFunction_GetFlags
PyCFunction_GetFunction=python39.PyCFunction_GetFunction
PyCFunction_GetSelf=python39.PyCFunction_GetSelf
Expand Down Expand Up @@ -584,7 +583,6 @@ EXPORTS
PyTraceBack_Print=python39.PyTraceBack_Print
PyTraceBack_Type=python39.PyTraceBack_Type DATA
PyTupleIter_Type=python39.PyTupleIter_Type DATA
PyTuple_ClearFreeList=python39.PyTuple_ClearFreeList
PyTuple_GetItem=python39.PyTuple_GetItem
PyTuple_GetSlice=python39.PyTuple_GetSlice
PyTuple_New=python39.PyTuple_New
Expand Down
Loading