Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

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
address comments
  • Loading branch information
Zekun Li committed Oct 1, 2017
commit fdd12ff84f4d30e259f7517d4fc9fbceca34c372
3 changes: 2 additions & 1 deletion Include/internal/mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ struct _gc_runtime_state {
/* linked lists of container objects */
struct gc_generation generations[NUM_GENERATIONS];
PyGC_Head *generation0;
/* a permanent generation which won't be collected */
struct gc_generation permanent_generation;
struct gc_generation_stats generation_stats[NUM_GENERATIONS];
/* true if we are currently running the collector */
int collecting;
Expand All @@ -185,7 +187,6 @@ struct _gc_runtime_state {
collections, and are awaiting to undergo a full collection for
the first time. */
Py_ssize_t long_lived_pending;
struct gc_generation permanent_generation;
};

PyAPI_FUNC(void) _PyGC_Initialize(struct _gc_runtime_state *);
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ def test_get_stats(self):

def test_freeze(self):
gc.freeze()
Copy link
Member

Choose a reason for hiding this comment

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

You should really undo the freezing after this test. We don't want tests to have so large side effects.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point! This will require adding the "thaw" or "unfreeze" function that will move objects back from the permanent generation to a properly collected one. Do you think it would be enough for that call to move all of them to gen0? This won't be exactly "undoing the freeze" but doesn't require keeping track of which object belonged to which generation before.

Copy link
Member

Choose a reason for hiding this comment

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

Anything that empties the permanent generation is good enough IMHO. People can call gc.collect explicitly afterwards if they want to collect those objects.

PS: as a non-native English locutor, I much prefer "unfreeze" to "thaw" :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, didn't notice your "general comment" until now. So it seems we agree that moving everything to a single generation makes sense. But you're suggesting the oldest generation (gen2) instead. Makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I was just too lazy to add it but fine will update..

self.assertGreater(gc.get_freeze_stats(), 0)
self.assertGreater(gc.get_freeze_count(), 0)


class GCCallbackTests(unittest.TestCase):
Expand Down
52 changes: 51 additions & 1 deletion Modules/clinic/gcmodule.c.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,4 +255,54 @@ PyDoc_STRVAR(gc_is_tracked__doc__,

#define GC_IS_TRACKED_METHODDEF \
{"is_tracked", (PyCFunction)gc_is_tracked, METH_O, gc_is_tracked__doc__},
/*[clinic end generated code: output=5a58583f00ab018e input=a9049054013a1b77]*/

PyDoc_STRVAR(gc_freeze__doc__,
"freeze($module, /)\n"
"--\n"
"\n"
"Freeze all current tracked objects and ignore them for future collections.\n"
"\n"
"This can be used before a fork to make the gc copy-on-write friendly.\n"
"Note: collection before a fork may free pages for future allocation\n"
"which can cause copy-on-write.");

#define GC_FREEZE_METHODDEF \
{"freeze", (PyCFunction)gc_freeze, METH_NOARGS, gc_freeze__doc__},

static PyObject *
gc_freeze_impl(PyObject *module);

static PyObject *
gc_freeze(PyObject *module, PyObject *Py_UNUSED(ignored))
{
return gc_freeze_impl(module);
}

PyDoc_STRVAR(gc_get_freeze_count__doc__,
"get_freeze_count($module, /)\n"
"--\n"
"\n"
"Return the number of objects in permanent generations.");

#define GC_GET_FREEZE_COUNT_METHODDEF \
{"get_freeze_count", (PyCFunction)gc_get_freeze_count, METH_NOARGS, gc_get_freeze_count__doc__},

static int
gc_get_freeze_count_impl(PyObject *module);

static PyObject *
gc_get_freeze_count(PyObject *module, PyObject *Py_UNUSED(ignored))
{
PyObject *return_value = NULL;
int _return_value;

_return_value = gc_get_freeze_count_impl(module);
if ((_return_value == -1) && PyErr_Occurred()) {
goto exit;
}
return_value = PyLong_FromLong((long)_return_value);

exit:
return return_value;
}
/*[clinic end generated code: output=004d91aafb1827f0 input=a9049054013a1b77]*/
44 changes: 24 additions & 20 deletions Modules/gcmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable,
for (i = 0; i < NUM_GENERATIONS; i++)
PySys_FormatStderr(" %zd",
gc_list_size(GEN_HEAD(i)));
PySys_WriteStderr("\ngc: objects in permanent generation: %d",
PySys_WriteStderr("\ngc: objects in permanent generation: %zd",
gc_list_size(&_PyRuntime.gc.permanent_generation.head));
t1 = _PyTime_GetMonotonicClock();

Expand Down Expand Up @@ -1411,17 +1411,19 @@ gc_is_tracked(PyObject *module, PyObject *obj)
return result;
}

PyDoc_STRVAR(gc_freeze__doc__,
"freeze() -> None\n"
"\n"
"Freeze all current tracked objects and ignore them for future collections.\n"
"This can be used before a fork to make the gc copy-on-write friendly.\n"
"Note: collection before a fork may free pages for future allocation\n"
"which can cause copy-on-write.\n"
);
/*[clinic input]
gc.freeze

Freeze all current tracked objects and ignore them for future collections.

This can be used before a fork to make the gc copy-on-write friendly.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than "a fork", perhaps use the wording "a POSIX fork() call", as that may not be obvious to all readers.

Note: collection before a fork may free pages for future allocation
which can cause copy-on-write.
[clinic start generated code]*/

static PyObject *
gc_freeze(PyObject *module)
gc_freeze_impl(PyObject *module)
/*[clinic end generated code: output=502159d9cdc4c139 input=8798c406cd01d7c4]*/
{
for (int i = 0; i < NUM_GENERATIONS; ++i) {
gc_list_merge(GEN_HEAD(i), &_PyRuntime.gc.permanent_generation.head);
Expand All @@ -1430,14 +1432,16 @@ gc_freeze(PyObject *module)
Py_RETURN_NONE;
}

PyDoc_STRVAR(gc_get_freeze_stats__doc__,
"get_freeze_stats() -> n\n"
"\n"
"Return the number of objects in permanent generations.\n"
);
/*[clinic input]
gc.get_freeze_count -> int

static PyObject *
gc_get_freeze_stats(PyObject *module) {
Return the number of objects in permanent generations.
[clinic start generated code]*/

static int
gc_get_freeze_count_impl(PyObject *module)
/*[clinic end generated code: output=e4e2ebcc77e5cbf3 input=590241a6a398cfa2]*/
{
return Py_BuildValue("i", gc_list_size(&_PyRuntime.gc.permanent_generation.head));
}

Expand All @@ -1460,7 +1464,7 @@ PyDoc_STRVAR(gc__doc__,
"get_referrers() -- Return the list of objects that refer to an object.\n"
"get_referents() -- Return the list of objects that an object refers to.\n"
"freeze() -- Freeze all tracked objects and ignore them for future collections.\n"
"get_freeze_stats() -- Return the number of objects in the permanent generation.\n");
"get_freeze_count() -- Return the number of objects in the permanent generation.\n");

static PyMethodDef GcMethods[] = {
GC_ENABLE_METHODDEF
Expand All @@ -1479,8 +1483,8 @@ static PyMethodDef GcMethods[] = {
gc_get_referrers__doc__},
{"get_referents", gc_get_referents, METH_VARARGS,
gc_get_referents__doc__},
{"freeze", gc_freeze, METH_NOARGS, gc_freeze__doc__},
{"get_freeze_stats", gc_get_freeze_stats, METH_NOARGS, gc_get_freeze_stats__doc__},
GC_FREEZE_METHODDEF
GC_GET_FREEZE_COUNT_METHODDEF
{NULL, NULL} /* Sentinel */
};

Expand Down