https://github.com/python/cpython/commit/6e9863d7a3516cc76d6ce13923b15620499f3855
commit: 6e9863d7a3516cc76d6ce13923b15620499f3855
branch: main
author: Irit Katriel <[email protected]>
committer: iritkatriel <[email protected]>
date: 2024-05-21T20:42:51Z
summary:
gh-118692: Avoid creating unnecessary StopIteration instances for monitoring
(#119216)
files:
A Misc/NEWS.d/next/Core and
Builtins/2024-05-20-14-57-39.gh-issue-118692.Qadm7F.rst
M Doc/c-api/monitoring.rst
M Include/cpython/monitoring.h
M Include/internal/pycore_opcode_metadata.h
M Lib/test/test_monitoring.py
M Modules/_testcapi/monitoring.c
M Python/bytecodes.c
M Python/ceval.c
M Python/generated_cases.c.h
M Python/instrumentation.c
diff --git a/Doc/c-api/monitoring.rst b/Doc/c-api/monitoring.rst
index 763ec8ef761e4e..ec743b98ba7024 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 efb9ec0e587552..797ba51246b1c6 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 2a237bc6dd8ee5..665b8627dfcc52 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 6974bc5517ae5f..b7c6abed1016dc 100644
--- a/Lib/test/test_monitoring.py
+++ b/Lib/test/test_monitoring.py
@@ -1938,19 +1938,28 @@ def setUp(self):
( 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 @@ def test_fire_event(self):
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 @@ def test_missing_exception(self):
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 00000000000000..11d177886df5b5
--- /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 aa90cfc06c1536..6fd4a405688f48 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 55eda9711dea1f..434eb804213810 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 128e0417a9fd63..324d062fe9bb43 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 8b8112209cc78a..96161c5a6586fd 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 77d3489afcfc72..3d78214738e66b 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,17 @@ _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);
+ return exception_event_teardown(err, NULL);
}
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
