Skip to content

gh-95909: Move PyArg_Parser.kwtuple to PyInterpreterState for Non-Core Cases #95910

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

4 changes: 3 additions & 1 deletion Include/cpython/modsupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ typedef struct _PyArg_Parser {
const char * const *keywords;
const char *fname;
const char *custom_msg;
int len; /* total number of parameters */
int pos; /* number of positional-only arguments */
int min; /* minimal number of arguments */
int max; /* maximal number of positional arguments */
PyObject *kwtuple; /* tuple of keyword parameter names */
PyObject *kwtuple; /* tuple of keyword parameter names
(or key into per-interper mapping) */
struct _PyArg_Parser *next;
} _PyArg_Parser;

Expand Down
7 changes: 7 additions & 0 deletions Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ struct _ceval_state {
};


struct getargs_state {
/* A dict mapping a pointer ID to a keywords tuple. */
PyObject *kwtuples;
};


// atexit state
typedef struct {
PyObject *func;
Expand Down Expand Up @@ -175,6 +181,7 @@ struct _is {
struct ast_state ast;
struct types_state types;
struct callable_cache callable_cache;
struct getargs_state getargs;

/* The following fields are here to avoid allocation during init.
The data is exposed through PyInterpreterState pointer fields.
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ extern void _PyAST_Fini(PyInterpreterState *interp);
extern void _PyAtExit_Fini(PyInterpreterState *interp);
extern void _PyThread_FiniType(PyInterpreterState *interp);
extern void _Py_Deepfreeze_Fini(void);
extern void _PyArg_Fini(void);
extern void _PyArg_Fini(PyInterpreterState *interp);

extern PyStatus _PyGILState_Init(_PyRuntimeState *runtime);
extern PyStatus _PyGILState_SetTstate(PyThreadState *tstate);
Expand Down
6 changes: 6 additions & 0 deletions Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ extern "C" {
#include "pycore_unicodeobject.h" // struct _Py_unicode_runtime_ids


struct getargs_runtime_state {
PyThread_type_lock mutex;
};

/* ceval state */

struct _ceval_runtime_state {
Expand Down Expand Up @@ -125,6 +129,8 @@ typedef struct pyruntimestate {

struct _Py_unicode_runtime_ids unicode_ids;

struct getargs_runtime_state getargs;

/* All the objects that are shared by the runtime's interpreters. */
struct _Py_global_objects global_objects;

Expand Down
199 changes: 151 additions & 48 deletions Python/getargs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
/* New getargs implementation */

#include "Python.h"
#include "pycore_object.h" // _PyObject_IMMORTAL_REFCNT
#include "pycore_pylifecycle.h" // _PyArg_Fini()
#include "pycore_pystate.h" // _Py_IsMainInterpreter()
#include "pycore_tuple.h" // _PyTuple_ITEMS()
#include "pycore_pylifecycle.h" // _PyArg_Fini

#include <ctype.h>
#include <float.h>
Expand Down Expand Up @@ -1952,45 +1954,130 @@ parse_format(const char *format, int total, int npos,
return 0;
}

static PyObject *
new_kwtuple(const char * const *keywords, int total, int pos)
#define KWTUPLE_IS_STATIC(parser) \
(PyTuple_CheckExact(parser->kwtuple))

static int
init_kwtuple(struct _PyArg_Parser *parser)
{
int nkw = total - pos;
PyObject *key;
if (parser->kwtuple != NULL) {
assert(!KWTUPLE_IS_STATIC(parser));
assert(PyLong_CheckExact(parser->kwtuple));
key = parser->kwtuple;
}
else {
if (parser->len == parser->pos) {
parser->kwtuple = (PyObject *)&_Py_SINGLETON(tuple_empty);
return 0;
}
// XXX This should be made "immortal"
key = PyLong_FromVoidPtr((void *)parser);
if (key == NULL) {
return -1;
}
Py_SET_REFCNT(key, _PyObject_IMMORTAL_REFCNT);
// We will leave this in place even if there's an error later.
parser->kwtuple = key;
}

/* Initialize the interpreter state, if necessary. */
PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->getargs.kwtuples == NULL) {
// We will leave this in place even if there's an error later.
interp->getargs.kwtuples = PyDict_New();
if (interp->getargs.kwtuples == NULL) {
return -1;
}
}

/* Initialize the tuple. */
int nkw = parser->len - parser->pos;
PyObject *kwtuple = PyTuple_New(nkw);
if (kwtuple == NULL) {
return NULL;
return -1;
}
keywords += pos;
const char * const *keywords = parser->keywords + parser->pos;
for (int i = 0; i < nkw; i++) {
PyObject *str = PyUnicode_FromString(keywords[i]);
if (str == NULL) {
Py_DECREF(kwtuple);
return NULL;
return -1;
}
PyUnicode_InternInPlace(&str);
PyTuple_SET_ITEM(kwtuple, i, str);
}
return kwtuple;

/* Add the tuple to the interpreter state. */
int res = PyDict_SetItem(interp->getargs.kwtuples, key, kwtuple);
Py_DECREF(kwtuple);
// We keep a reference to key in parser->kwtuple.
if (res < 0) {
return -1;
}
return 0;
}

static int
parser_init(struct _PyArg_Parser *parser)
static PyObject *
get_kwtuple(struct _PyArg_Parser *parser)
{
const char * const *keywords = parser->keywords;
assert(keywords != NULL);
assert(parser->kwtuple != NULL);
if (KWTUPLE_IS_STATIC(parser)) {
assert(PyTuple_CheckExact(parser->kwtuple));
return parser->kwtuple;
}
/* It was initialized at runtime on the interpreter state. */
PyInterpreterState *interp = _PyInterpreterState_GET();
assert(interp->getargs.kwtuples != NULL);
assert(PyLong_CheckExact(parser->kwtuple));
return PyDict_GetItem(interp->getargs.kwtuples, parser->kwtuple);
}

if (parser->initialized) {
assert(parser->kwtuple != NULL);
return 1;
static void
clear_kwtuple(struct _PyArg_Parser *parser)
{
// This should be called only at the end of runtime finalization.
assert(_Py_IsMainInterpreter(_PyInterpreterState_GET()));
if (parser->kwtuple != NULL && !KWTUPLE_IS_STATIC(parser)) {
PyObject *key = parser->kwtuple;
assert(PyLong_CheckExact(key));
// We don't expose parser->kwtuple anywhere
// so we can control the refcount.
assert(Py_REFCNT(key) == _PyObject_IMMORTAL_REFCNT);
Py_SET_REFCNT(key, 1);
Py_CLEAR(parser->kwtuple);
}
}

static void
clear_kwtuples(PyInterpreterState *interp)
{
#ifdef Py_DEBUG
if (interp->getargs.kwtuples != NULL) {
// Nothing else should be holding a reference to any of the tuples.
Py_ssize_t i = 0;
PyObject *kwtuple;
while (PyDict_Next(interp->getargs.kwtuples, &i, NULL, &kwtuple)) {
assert(Py_REFCNT(kwtuple) == 1);
}
}
assert(parser->pos == 0 &&
#endif // Py_DEBUG
Py_CLEAR(interp->getargs.kwtuples);
}

static int
_parser_init(struct _PyArg_Parser *parser)
{
assert(parser->len == 0 &&
parser->pos == 0 &&
(parser->format == NULL || parser->fname == NULL) &&
parser->custom_msg == NULL &&
parser->min == 0 &&
parser->max == 0);
parser->max == 0 &&
(parser->kwtuple == NULL || KWTUPLE_IS_STATIC(parser)));

int len, pos;
if (scan_keywords(keywords, &len, &pos) < 0) {
if (scan_keywords(parser->keywords, &len, &pos) < 0) {
return 0;
}

Expand All @@ -2008,39 +2095,52 @@ parser_init(struct _PyArg_Parser *parser)
fname = parser->fname;
}

int owned;
PyObject *kwtuple = parser->kwtuple;
if (kwtuple == NULL) {
kwtuple = new_kwtuple(keywords, len, pos);
if (kwtuple == NULL) {
return 0;
}
owned = 1;
}
else {
owned = 0;
}

parser->len = len;
parser->pos = pos;
parser->fname = fname;
parser->custom_msg = custommsg;
parser->min = min;
parser->max = max;
parser->kwtuple = kwtuple;
parser->initialized = owned ? 1 : -1;
if (parser->kwtuple == NULL) {
if (init_kwtuple(parser) < 0) {
return 0;
}
}
parser->initialized = 1;

assert(parser->next == NULL);
parser->next = static_arg_parsers;
static_arg_parsers = parser;
return 1;
}

static int
parser_init(struct _PyArg_Parser *parser)
{
int res = 1;
assert(parser->keywords != NULL);

PyThread_acquire_lock(_PyRuntime.getargs.mutex, WAIT_LOCK);
if (parser->initialized) {
PyThread_release_lock(_PyRuntime.getargs.mutex);
assert(parser->kwtuple != NULL);
if (!KWTUPLE_IS_STATIC(parser)) {
if (init_kwtuple(parser) < 0) {
res = 0;
}
}
}
else {
res = _parser_init(parser);
PyThread_release_lock(_PyRuntime.getargs.mutex);
}
return res;
}

static void
parser_clear(struct _PyArg_Parser *parser)
{
if (parser->initialized == 1) {
Py_CLEAR(parser->kwtuple);
}
clear_kwtuple(parser);
}

static PyObject*
Expand Down Expand Up @@ -2110,7 +2210,7 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs,
return 0;
}

kwtuple = parser->kwtuple;
kwtuple = get_kwtuple(parser);
pos = parser->pos;
len = pos + (int)PyTuple_GET_SIZE(kwtuple);

Expand Down Expand Up @@ -2342,7 +2442,7 @@ _PyArg_UnpackKeywords(PyObject *const *args, Py_ssize_t nargs,
return NULL;
}

kwtuple = parser->kwtuple;
kwtuple = get_kwtuple(parser);
posonly = parser->pos;
minposonly = Py_MIN(posonly, minpos);
maxargs = posonly + (int)PyTuple_GET_SIZE(kwtuple);
Expand Down Expand Up @@ -2518,7 +2618,7 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs,
return NULL;
}

kwtuple = parser->kwtuple;
kwtuple = get_kwtuple(parser);
posonly = parser->pos;
minposonly = Py_MIN(posonly, minpos);
maxargs = posonly + (int)PyTuple_GET_SIZE(kwtuple);
Expand Down Expand Up @@ -2913,16 +3013,19 @@ _PyArg_NoKwnames(const char *funcname, PyObject *kwnames)
}

void
_PyArg_Fini(void)
_PyArg_Fini(PyInterpreterState *interp)
{
struct _PyArg_Parser *tmp, *s = static_arg_parsers;
while (s) {
tmp = s->next;
s->next = NULL;
parser_clear(s);
s = tmp;
}
static_arg_parsers = NULL;
clear_kwtuples(interp);
if (_Py_IsMainInterpreter(interp)) {
struct _PyArg_Parser *tmp, *s = static_arg_parsers;
while (s) {
tmp = s->next;
s->next = NULL;
parser_clear(s);
s = tmp;
}
static_arg_parsers = NULL;
}
}

#ifdef __cplusplus
Expand Down
6 changes: 5 additions & 1 deletion Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -1717,7 +1717,11 @@ finalize_interp_clear(PyThreadState *tstate)

if (is_main_interp) {
_Py_HashRandomization_Fini();
_PyArg_Fini();
}

_PyArg_Fini(tstate->interp);

if (is_main_interp) {
_Py_ClearFileSystemEncoding();
_Py_Deepfreeze_Fini();
}
Expand Down
Loading