While I'm looking Python's memory usage, I found some dicts which key
is not interned "_fields", "_ast" and "__modules__".

Python/Python-ast.c started with:

/* File automatically generated by Parser/asdl_c.py. */

Please fix the generator, not the generated file ;-)

> While I'm looking Python's memory usage, I found some dicts which key is not interned "_fields", "_ast" and "__modules__".

AST objects are supposed to be temporary. Interning strings used in the AST processor would make these strings immortal and so increase the memory usage, no?

What is the purpose of the patch? Speedup or reduce the memory usage?

LGTM. _Py_IDENTIFIER is private API, but is not new in Python-ast.c.

I expect insignificant speedup and reduce the memory usage from the patch.

> AST objects are supposed to be temporary. Interning strings used in the AST processor would make these strings immortal and so increase the memory usage, no?
> What is the purpose of the patch? Speedup or reduce the memory usage?

The three identifiers are used in type.  Dozen (100+) types using them.

>>> import _ast
>>> _ast.Dict.__dict__
mappingproxy({'_fields': ('keys', 'values'), '__module__': '_ast', '__doc__': None})
>>> _ast.DictComp.__dict__
mappingproxy({'_fields': ('key', 'value', 'generators'), '__module__': '_ast', '__doc__': None})


Each string uses about 50 bytes. So 50 * 3 * 100 = 15KB may be reduced.

But main purpose is less noise when looking memory usage of Python.

New changeset a507882736b2 by INADA Naoki in branch 'default':
Issue #29369: Use Py_IDENTIFIER in Python-ast.c
https://hg.python.org/cpython/rev/a507882736b2

+    result = PyObject_CallFunction((PyObject*)&PyType_Type, "s(O){OOOO}",
+                    type, base,
+                    _PyUnicode_FromId(&PyId__fields), fnames,
+                    _PyUnicode_FromId(&PyId___module__),
+                    _PyUnicode_FromId(&PyId__ast));

You should check if _PyUnicode_FromId() returns NULL if it was the first call and the UTF-8 decode failed to allocate memory.

You might initialize all these identifiers (and check for errors) in init_types() to avoid having to check for errors each time they are used.

> The three identifiers are used in type.  Dozen (100+) types using them.

I tried once to put *all* (most common) _Py_IDENTIFIER() in a single file and share them, but I abandoned my attempt:

http://bugs.python.org/issue19515#msg202385
Antoine Pitrou: "Well, then IMHO it's not worth it."

See issues #19512 and #19515.


"""
>>> import _ast
>>> _ast.Dict.__dict__
mappingproxy({'_fields': ('keys', 'values'), '__module__': '_ast', '__doc__': None})
"""

But the _ast module is not imported by default, only in programs importing "ast" explicitly.

Well, I'm not opposed to the change, I'm just trying to understand how the code is used ;-)

> You should check if _PyUnicode_FromId() returns NULL if it was the first call and the UTF-8 decode failed to allocate memory.

thanks. new patch will fix it.

> You might initialize all these identifiers (and check for errors) in init_types() to avoid having to check for errors each time they are used.

Here is not so performance critical part.

> But the _ast module is not imported by default, only in programs importing "ast" explicitly.
> Well, I'm not opposed to the change, I'm just trying to understand how the code is used ;-)

We use Flask. Flask is based on Werkzeug. Werkzeug imports inspect.
inspect imports ast. ast imports _ast.

"We use Flask. Flask is based on Werkzeug. Werkzeug imports inspect. inspect imports ast. ast imports _ast."

Oh, this was unexpected to me :-)

I think additional checks are not needed. PyObject_CallFunction() allows passing NULLs.

Thanks, Serhiy.  You're right.