From 91fb626a06fa4e00b537eb80d612368dcdaadc0a Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 2 Jul 2024 12:30:14 -0400 Subject: [PATCH] gh-117139: Add _PyTuple_FromStackRefSteal and use it (#121244) Avoids the extra conversion from stack refs to PyObjects. --- Include/internal/pycore_stackref.h | 2 +- Include/internal/pycore_tuple.h | 1 + Objects/tupleobject.c | 21 +++++++++++++++++++++ Python/bytecodes.c | 8 +------- Python/ceval.c | 8 +------- Python/executor_cases.c.h | 10 +--------- Python/generated_cases.c.h | 10 +--------- Tools/cases_generator/analyzer.py | 1 + 8 files changed, 28 insertions(+), 33 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 32e445dd96f9a16..4301c6a7cb40b0a 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -48,7 +48,7 @@ extern "C" { CPython refcounting operations on it! */ -typedef union { +typedef union _PyStackRef { uintptr_t bits; } _PyStackRef; diff --git a/Include/internal/pycore_tuple.h b/Include/internal/pycore_tuple.h index 14a9e42c3a324cd..dfbbd6fd0c7de56 100644 --- a/Include/internal/pycore_tuple.h +++ b/Include/internal/pycore_tuple.h @@ -21,6 +21,7 @@ extern PyStatus _PyTuple_InitGlobalObjects(PyInterpreterState *); #define _PyTuple_ITEMS(op) _Py_RVALUE(_PyTuple_CAST(op)->ob_item) extern PyObject *_PyTuple_FromArray(PyObject *const *, Py_ssize_t); +PyAPI_FUNC(PyObject *)_PyTuple_FromStackRefSteal(const union _PyStackRef *, Py_ssize_t); PyAPI_FUNC(PyObject *)_PyTuple_FromArraySteal(PyObject *const *, Py_ssize_t); typedef struct { diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 5ae1ee9a89af84c..994258f20b495df 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -390,6 +390,27 @@ _PyTuple_FromArray(PyObject *const *src, Py_ssize_t n) return (PyObject *)tuple; } +PyObject * +_PyTuple_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n) +{ + if (n == 0) { + return tuple_get_empty(); + } + PyTupleObject *tuple = tuple_alloc(n); + if (tuple == NULL) { + for (Py_ssize_t i = 0; i < n; i++) { + PyStackRef_CLOSE(src[i]); + } + return NULL; + } + PyObject **dst = tuple->ob_item; + for (Py_ssize_t i = 0; i < n; i++) { + dst[i] = PyStackRef_AsPyObjectSteal(src[i]); + } + _PyObject_GC_TRACK(tuple); + return (PyObject *)tuple; +} + PyObject * _PyTuple_FromArraySteal(PyObject *const *src, Py_ssize_t n) { diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 343481e9313de48..76587a4f0dc6951 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1780,13 +1780,7 @@ dummy_func( } inst(BUILD_TUPLE, (values[oparg] -- tup)) { - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - DECREF_INPUTS(); - ERROR_IF(true, error); - } - PyObject *tup_o = _PyTuple_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); ERROR_IF(tup_o == NULL, error); tup = PyStackRef_FromPyObjectSteal(tup_o); } diff --git a/Python/ceval.c b/Python/ceval.c index a71244676f3029a..a240ed4321f7ee0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1500,13 +1500,7 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func, u = (PyObject *)&_Py_SINGLETON(tuple_empty); } else { - assert(args != NULL); - STACKREFS_TO_PYOBJECTS((_PyStackRef *)args, argcount, args_o); - if (args_o == NULL) { - goto fail_pre_positional; - } - u = _PyTuple_FromArraySteal((args_o + n), argcount - n); - STACKREFS_TO_PYOBJECTS_CLEANUP(args_o); + u = _PyTuple_FromStackRefSteal(args + n, argcount - n); } if (u == NULL) { goto fail_post_positional; diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index d70a57a9a8ffbeb..3b999465aac815d 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1816,15 +1816,7 @@ _PyStackRef tup; oparg = CURRENT_OPARG(); values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - for (int _i = oparg; --_i >= 0;) { - PyStackRef_CLOSE(values[_i]); - } - if (true) JUMP_TO_ERROR(); - } - PyObject *tup_o = _PyTuple_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); if (tup_o == NULL) JUMP_TO_ERROR(); tup = PyStackRef_FromPyObjectSteal(tup_o); stack_pointer[-oparg] = tup; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 32b22aff14a7681..61057221291c0a4 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -769,15 +769,7 @@ _PyStackRef *values; _PyStackRef tup; values = &stack_pointer[-oparg]; - STACKREFS_TO_PYOBJECTS(values, oparg, values_o); - if (CONVERSION_FAILED(values_o)) { - for (int _i = oparg; --_i >= 0;) { - PyStackRef_CLOSE(values[_i]); - } - if (true) { stack_pointer += -oparg; goto error; } - } - PyObject *tup_o = _PyTuple_FromArraySteal(values_o, oparg); - STACKREFS_TO_PYOBJECTS_CLEANUP(values_o); + PyObject *tup_o = _PyTuple_FromStackRefSteal(values, oparg); if (tup_o == NULL) { stack_pointer += -oparg; goto error; } tup = PyStackRef_FromPyObjectSteal(tup_o); stack_pointer[-oparg] = tup; diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 6b1af1b59f14d82..f92560bd2b76b30 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -431,6 +431,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "CONVERSION_FAILED", "_PyList_FromArraySteal", "_PyTuple_FromArraySteal", + "_PyTuple_FromStackRefSteal", ) ESCAPING_FUNCTIONS = (