From c864efba25465eb6a4fff7e0a6df80a9ba449370 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Fri, 24 May 2024 12:18:13 +0200 Subject: [3.13] gh-118692: Avoid creating unnecessary StopIteration instances for monitoring (GH-119216) (#119497) * gh-118692: Avoid creating unnecessary StopIteration instances for monitoring (GH-119216) (cherry picked from commit 6e9863d7a3516cc76d6ce13923b15620499f3855) --------- Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Doc/c-api/monitoring.rst | 6 ++--- Include/cpython/monitoring.h | 6 ++--- Include/internal/pycore_opcode_metadata.h | 4 +-- Lib/test/test_monitoring.py | 31 ++++++++++++++++++---- .../2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst | 1 + Modules/_testcapi/monitoring.c | 9 ++++--- Python/bytecodes.c | 8 ++---- Python/ceval.c | 14 +++++++--- Python/generated_cases.c.h | 8 ++---- Python/instrumentation.c | 9 ++++--- 10 files changed, 61 insertions(+), 35 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst diff --git a/Doc/c-api/monitoring.rst b/Doc/c-api/monitoring.rst index 763ec8e..ec743b9 100644 --- a/Doc/c-api/monitoring.rst +++ b/Doc/c-api/monitoring.rst @@ -121,10 +121,10 @@ See :mod:`sys.monitoring` for descriptions of the events. :c:func:`PyErr_GetRaisedException`). -.. c:function:: int PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset) +.. c:function:: int PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value) - Fire a ``STOP_ITERATION`` event with the current exception (as returned by - :c:func:`PyErr_GetRaisedException`). + Fire a ``STOP_ITERATION`` event. If ``value`` is an instance of :exc:`StopIteration`, it is used. Otherwise, + a new :exc:`StopIteration` instance is created with ``value`` as its argument. Managing the Monitoring State diff --git a/Include/cpython/monitoring.h b/Include/cpython/monitoring.h index efb9ec0..797ba51 100644 --- a/Include/cpython/monitoring.h +++ b/Include/cpython/monitoring.h @@ -101,7 +101,7 @@ PyAPI_FUNC(int) _PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset); PyAPI_FUNC(int) -_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset); +_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value); #define _PYMONITORING_IF_ACTIVE(STATE, X) \ @@ -240,11 +240,11 @@ PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, int } static inline int -PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset) +PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value) { _PYMONITORING_IF_ACTIVE( state, - _PyMonitoring_FireStopIterationEvent(state, codelike, offset)); + _PyMonitoring_FireStopIterationEvent(state, codelike, offset, value)); } #undef _PYMONITORING_IF_ACTIVE diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 2a237bc..665b862 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1060,8 +1060,8 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[268] = { [INSTRUMENTED_CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_CALL_FUNCTION_EX] = { true, INSTR_FMT_IX, 0 }, [INSTRUMENTED_CALL_KW] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [INSTRUMENTED_END_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, - [INSTRUMENTED_END_SEND] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, + [INSTRUMENTED_END_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, + [INSTRUMENTED_END_SEND] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG }, [INSTRUMENTED_FOR_ITER] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_INSTRUCTION] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_JUMP_BACKWARD] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG }, diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index 6974bc5..b7c6abe 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1938,19 +1938,28 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase): ( 1, E.RAISE, capi.fire_event_raise, ValueError(2)), ( 1, E.EXCEPTION_HANDLED, capi.fire_event_exception_handled, ValueError(5)), ( 1, E.PY_UNWIND, capi.fire_event_py_unwind, ValueError(6)), - ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, ValueError(7)), + ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, 7), + ( 1, E.STOP_ITERATION, capi.fire_event_stop_iteration, StopIteration(8)), ] + self.EXPECT_RAISED_EXCEPTION = [E.PY_THROW, E.RAISE, E.EXCEPTION_HANDLED, E.PY_UNWIND] - def check_event_count(self, event, func, args, expected): + + def check_event_count(self, event, func, args, expected, callback_raises=None): class Counter: - def __init__(self): + def __init__(self, callback_raises): + self.callback_raises = callback_raises self.count = 0 + def __call__(self, *args): self.count += 1 + if self.callback_raises: + exc = self.callback_raises + self.callback_raises = None + raise exc try: - counter = Counter() + counter = Counter(callback_raises) sys.monitoring.register_callback(TEST_TOOL, event, counter) if event == E.C_RETURN or event == E.C_RAISE: sys.monitoring.set_events(TEST_TOOL, E.CALL) @@ -1987,8 +1996,9 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase): def test_missing_exception(self): for _, event, function, *args in self.cases: - if not (args and isinstance(args[-1], BaseException)): + if event not in self.EXPECT_RAISED_EXCEPTION: continue + assert args and isinstance(args[-1], BaseException) offset = 0 self.codelike = _testcapi.CodeLike(1) with self.subTest(function.__name__): @@ -1997,6 +2007,17 @@ class TestCApiEventGeneration(MonitoringTestBase, unittest.TestCase): expected = ValueError(f"Firing event {evt} with no exception set") self.check_event_count(event, function, args_, expected) + def test_fire_event_failing_callback(self): + for expected, event, function, *args in self.cases: + offset = 0 + self.codelike = _testcapi.CodeLike(1) + with self.subTest(function.__name__): + args_ = (self.codelike, offset) + tuple(args) + exc = OSError(42) + with self.assertRaises(type(exc)): + self.check_event_count(event, function, args_, expected, + callback_raises=exc) + CANNOT_DISABLE = { E.PY_THROW, E.RAISE, E.RERAISE, E.EXCEPTION_HANDLED, E.PY_UNWIND } diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst new file mode 100644 index 0000000..11d1778 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst @@ -0,0 +1 @@ +Avoid creating unnecessary :exc:`StopIteration` instances for monitoring. diff --git a/Modules/_testcapi/monitoring.c b/Modules/_testcapi/monitoring.c index aa90cfc..6fd4a40 100644 --- a/Modules/_testcapi/monitoring.c +++ b/Modules/_testcapi/monitoring.c @@ -416,16 +416,17 @@ fire_event_stop_iteration(PyObject *self, PyObject *args) { PyObject *codelike; int offset; - PyObject *exception; - if (!PyArg_ParseTuple(args, "OiO", &codelike, &offset, &exception)) { + PyObject *value; + if (!PyArg_ParseTuple(args, "OiO", &codelike, &offset, &value)) { return NULL; } - NULLABLE(exception); + NULLABLE(value); + PyObject *exception = NULL; PyMonitoringState *state = setup_fire(codelike, offset, exception); if (state == NULL) { return NULL; } - int res = PyMonitoring_FireStopIterationEvent(state, codelike, offset); + int res = PyMonitoring_FireStopIterationEvent(state, codelike, offset, value); RETURN_INT(teardown_fire(res, state, exception)); } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 55eda97..434eb80 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -292,11 +292,9 @@ dummy_func( /* Need to create a fake StopIteration error here, * to conform to PEP 380 */ if (PyGen_Check(receiver)) { - PyErr_SetObject(PyExc_StopIteration, value); - if (monitor_stop_iteration(tstate, frame, this_instr)) { + if (monitor_stop_iteration(tstate, frame, this_instr, value)) { ERROR_NO_POP(); } - PyErr_SetRaisedException(NULL); } DECREF_INPUTS(); } @@ -307,11 +305,9 @@ dummy_func( tier1 inst(INSTRUMENTED_END_SEND, (receiver, value -- value)) { if (PyGen_Check(receiver) || PyCoro_CheckExact(receiver)) { - PyErr_SetObject(PyExc_StopIteration, value); - if (monitor_stop_iteration(tstate, frame, this_instr)) { + if (monitor_stop_iteration(tstate, frame, this_instr, value)) { ERROR_NO_POP(); } - PyErr_SetRaisedException(NULL); } Py_DECREF(receiver); } diff --git a/Python/ceval.c b/Python/ceval.c index 128e041..324d062 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -231,7 +231,8 @@ static void monitor_reraise(PyThreadState *tstate, _Py_CODEUNIT *instr); static int monitor_stop_iteration(PyThreadState *tstate, _PyInterpreterFrame *frame, - _Py_CODEUNIT *instr); + _Py_CODEUNIT *instr, + PyObject *value); static void monitor_unwind(PyThreadState *tstate, _PyInterpreterFrame *frame, _Py_CODEUNIT *instr); @@ -2215,12 +2216,19 @@ monitor_reraise(PyThreadState *tstate, _PyInterpreterFrame *frame, static int monitor_stop_iteration(PyThreadState *tstate, _PyInterpreterFrame *frame, - _Py_CODEUNIT *instr) + _Py_CODEUNIT *instr, PyObject *value) { if (no_tools_for_local_event(tstate, frame, PY_MONITORING_EVENT_STOP_ITERATION)) { return 0; } - return do_monitor_exc(tstate, frame, instr, PY_MONITORING_EVENT_STOP_ITERATION); + assert(!PyErr_Occurred()); + PyErr_SetObject(PyExc_StopIteration, value); + int res = do_monitor_exc(tstate, frame, instr, PY_MONITORING_EVENT_STOP_ITERATION); + if (res < 0) { + return res; + } + PyErr_SetRaisedException(NULL); + return 0; } static void diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 8b81122..96161c5 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3260,11 +3260,9 @@ /* Need to create a fake StopIteration error here, * to conform to PEP 380 */ if (PyGen_Check(receiver)) { - PyErr_SetObject(PyExc_StopIteration, value); - if (monitor_stop_iteration(tstate, frame, this_instr)) { + if (monitor_stop_iteration(tstate, frame, this_instr, value)) { goto error; } - PyErr_SetRaisedException(NULL); } Py_DECREF(value); stack_pointer += -1; @@ -3281,11 +3279,9 @@ value = stack_pointer[-1]; receiver = stack_pointer[-2]; if (PyGen_Check(receiver) || PyCoro_CheckExact(receiver)) { - PyErr_SetObject(PyExc_StopIteration, value); - if (monitor_stop_iteration(tstate, frame, this_instr)) { + if (monitor_stop_iteration(tstate, frame, this_instr, value)) { goto error; } - PyErr_SetRaisedException(NULL); } Py_DECREF(receiver); stack_pointer[-2] = value; diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 77d3489..9095fb9 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -2622,7 +2622,7 @@ exception_event_teardown(int err, PyObject *exc) { } else { assert(PyErr_Occurred()); - Py_DECREF(exc); + Py_XDECREF(exc); } return err; } @@ -2712,15 +2712,18 @@ _PyMonitoring_FirePyUnwindEvent(PyMonitoringState *state, PyObject *codelike, in } int -_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset) +_PyMonitoring_FireStopIterationEvent(PyMonitoringState *state, PyObject *codelike, int32_t offset, PyObject *value) { int event = PY_MONITORING_EVENT_STOP_ITERATION; assert(state->active); + assert(!PyErr_Occurred()); + PyErr_SetObject(PyExc_StopIteration, value); PyObject *exc; if (exception_event_setup(&exc, event) < 0) { return -1; } PyObject *args[4] = { NULL, NULL, NULL, exc }; int err = capi_call_instrumentation(state, codelike, offset, args, 3, event); - return exception_event_teardown(err, exc); + Py_DECREF(exc); + return exception_event_teardown(err, NULL); } -- cgit v0.12