Add a new private API clear private variables, which are initialized once, at Python shutdown

[vstinner]

BPO	29881
Nosy	@ncoghlan, @vstinner, @ericsnowcurrently, @serhiy-storchaka, @mlouielu
PRs	bpo-29881: Add new C API for static variables #770 bpo-29881: Add new C API for variables init once #780

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-09-19.23:11:03.288>
created_at = <Date 2017-03-22.16:43:29.613>
labels = ['interpreter-core', '3.7', 'performance']
title = 'Add a new private API clear private variables, which are initialized once, at Python shutdown'
updated_at = <Date 2018-09-19.23:11:03.288>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2018-09-19.23:11:03.288>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-19.23:11:03.288>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-03-22.16:43:29.613>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 29881
keywords = []
message_count = 30.0
messages = ['289999', '290002', '290018', '290026', '290034', '290036', '290037', '290038', '290039', '290040', '290041', '290043', '290045', '290046', '290047', '290048', '290049', '290052', '290056', '290058', '290078', '290079', '290869', '290891', '290893', '290897', '291430', '291542', '291545', '301377']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'vstinner', 'eric.snow', 'serhiy.storchaka', 'louielu']
pr_nums = ['770', '780']
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue29881'
versions = ['Python 3.7']

[vstinner]

When I read Serhiy Storshaka's idea in issue bpo-29878, it recalled me an old idea of writing a generalization of the _Py_IDENTIFIER() API.

_Py_IDENTIFIER is an API to initialize a static string variable once. The _Py_Identifier structure has a "next" field to create a single-linked chained list. It allows to clear all variables at exit in _PyUnicode_ClearStaticStrings().

I propose a similar API but for any PyObject* object, to be able to clear all static variables at exit. It should help to release all memory in Py_Finalize() and have a safer Python finalization.

See attached pull request for the API itself.

"Static variables" in C are variables with a limited scope: a single C file or a single function.

It seems like the API can remove some lines of code. Example of patch:

@@ -1452,14 +1450,14 @@ compiler_mod(struct compiler *c, mod_ty mod)
 {
     PyCodeObject *co;
     int addNone = 1;
-    static PyObject *module;
-    if (!module) {
-        module = PyUnicode_InternFromString("<module>");
-        if (!module)
-            return NULL;
+    _Py_STATICVAR(module);
+
+    if (_PY_STATICVAR_INIT(&module, PyUnicode_InternFromString("<module>"))) {
+        return 0;
     }
+
     /* Use 0 for firstlineno initially, will fixup in assemble(). */
-    if (!compiler_enter_scope(c, module, COMPILER_SCOPE_MODULE, mod, 0))
+    if (!compiler_enter_scope(c, module.obj, COMPILER_SCOPE_MODULE, mod, 0))
         return NULL;
     switch (mod->kind) {
     case Module_kind:

--

Drawbacks of the API:

• It adds one pointer per static variables, so increase the memory footprint of 8 bytes per variable
• It requires to write "var.obj" instead of just "var" to access the Python object

The API doesn't try to remove duplicated objects. I consider that it's not an issue since functions like PyLong_FromLong(<small int>) and PyUnicode_InternFromString("c string") already do it for us. Some functions create mutable variables like PyImport_Import() which creates an empty list.

--

Note: Eric Snow proposed a solution "solving multi-core Python":

• https://mail.python.org/pipermail/python-ideas/2015-June/034177.html
• http://ericsnowcurrently.blogspot.fr/2016/09/solving-mutli-core-python.html

I'm not sure if this API would help or not to implement such idea, but Eric's project is experimental and wasn't taken in account when designing the API.

[serhiy-storchaka]

The problem with existing static variables are that they are not properly cleared. When the Python interpreter is finalized and reinitialized they can contain invalid references. This patch fixes this issue.

• It requires to write "var.obj" instead of just "var" to access the Python object

You can use a dynamic array of PyObject** instead of a linked list for collecting references to "static variables" or use gc_next/gc_refs for managing a linked list.

[vstinner]

The purpose of the issue is to fix memory leaks like issue bpo-21387 (this one is not the best example since it seems like the leak doesn't come from a static variable.)

[serhiy-storchaka]

The patch contains an example of using _Py_STATICVAR(). But it doesn't use _PY_STATICVAR_INIT(). _PY_STATICVAR_INIT() can be used if extract the code of getting _array_reconstructor into separate function.

I like the idea in general, but I want to see more examples. Ideally -- the patch should use a new API for *all* static PyObject* variables. This will help to design good API.

[ncoghlan]

I like this idea in principle, and suspect it may be helpful in the implementation of the new thread-specific-storage API proposed in PEP-539 (also see http://bugs.python.org/issue25658 ).

How would you feel about calling it _Py_ONCE_VAR? The use case here is quite similar to the concepts behind the pthread_once API (just with the lifecycle tied to Py_Initialize/Py_Finalize rather than the C level process), and it's the "initialise-on-first-use" behaviour that's significant here, moreso than the fact that the typical storage target is a static variable.

As far as the linked list goes, the nice aspect of Victor's approach is that it doesn't need to do any additional runtime memory allocations - all the storage involved is still statically allocated in the modules that initialise the values, there are just some extra pointer assignments to link everything together (with the GIL protecting against race conditions).

However, the user visible ".obj" indirection could still be avoided at the cost of an additional pointer per entry:

    typedef struct _Py_OnceVar {
        struct _Py_OnceVar *next;
        PyObject **obj_ref;
    } _Py_OnceVar;

    #define _Py_ONCE_VAR(var_decl, var) \
      var_decl var = NULL;
      static _Py_OnceVar var ## _once_meta = {.next = NULL, .obj_ref = (PyObject **) &var}

Intended declaration:

_Py_ONCE_VAR(static PyObject *, array_reconstructor);

_Py_ONCE_VAR_INIT would similarly be adjusted to assign the supplied value to "var", while also setting "var_once_meta.next" to hook the value into the linked list.

[vstinner]

Serhiy: "The patch contains an example of using _Py_STATICVAR(). But it doesn't use _PY_STATICVAR_INIT()."

I'm not sure that I understand your comment. All modified code use _Py_STATICVAR() to declare the declare. All modified code use _PY_STATICVAR_INIT() to initialize the variable, except of array_array___reduce_ex__() which uses explicitly _PyStaticVar_Set() beause its initialization code is more complex.

_PyStaticVar_Set() is the low-level function, I would prefer to avoid it since it fails with an assertion error if it's called twice. _PY_STATICVAR_INIT() helper should be perfer since it makes the code shorter, but it's similar to simple expressions like PyUnicode_FromString("\n").

I tried to include examples of usage of both APIs to give you an idea of the API.

I chose to *not* patch the whole Python code base, because I would like to first get a review of a the API. It seems like alternatives have been proposed ;-)

I like the idea in general, but I want to see more examples.

Which kind of other examples do you need?

[vstinner]

#define _Py_ONCE_VAR(var_decl, var) \
      var_decl var = NULL;
      static _Py_OnceVar var ## _once_meta = {.next = NULL, .obj_ref = (PyObject **) &var}

Yeah, I had a similar idea, but I fear that it will increase the usage of the C stack memory.

See the issue bpo-28858: my old _PyObject_CallArg1() macro allocated an implicit array on the stack and increased the stack usage, whereas the purpose of the macro was to *reduce* the stack usage (just the opposite!). So I removed the macro.

[vstinner]

Serhiy: "use gc_next/gc_refs for managing a linked list"

I'm not sure that I understand your idea. These fields are already used internally by the garbage collector. If I modify one of these fields, it would corrupt the GC, no?

Serhiy: "You can use a dynamic array of PyObject** instead of a linked list for collecting references to "static variables""

Ah yes, I like the idea of using a single array to track all variables. An array reduces memory fragmentation and is simple to maintain, since the API only needs two operations: list.append(obj) and list.clear(). The API already requires to check for errors, so another memory allocation failure wouldn't be suprising.

[serhiy-storchaka]

My apologies Victor. It seems that the browser on my netbook shown only the part of changes and I seen only the change in array_array___reduce_ex__().

I like Nick's idea for hiding the indirection, but passing var_decl doesn't look having much sense to me. It should always be "static PyObject *".

I don't have a preference for the name, but the purpose of a new API is not just to ensure that a piece of initialization code is executed at most once, but that the object will be destroyed and the reference will be cleared after finalizing the interpreter. Seems this is different from the purpose of pthread_once().

[vstinner]

I wrote a new different API: #780

New C API for variables only initialized once to be able to clear
them at exit:

New macro _Py_ONCEVAR(var) to declare a variable
New macro _PY_ONCEVAR_INIT(var, expr) to initialize a variable
once
New function _PyOnceVar_Set() to explicitly set a variable once
to initialize it
New _PyOnceVar_Fini() function clearing all variables (initialized
once) at exit

Variables keep their PyObject* type, but the API hides an internal C array tracking all Python objects to Py_DECREF them at exit in _PyOnceVar_Fini().

I used Nick's naming scheme since it seems like pthread has an existing API which is similar, but different (my API doesn't require an initialization callback).

I really prefer the second API using PyObject*.

[serhiy-storchaka]

"use gc_next/gc_refs for managing a linked list"

Forgot about this idea. Not all objects have the GC header.

But if they have it they could be removed from the GC list and added in the separate list since in any case the garbage collector shouldn't destroy objects referenced by static variables.

[serhiy-storchaka]

The new API rather combines pthread_once() and pthread_cleanup_push().