Skip to content
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

[3.13] gh-118692: Avoid creating unnecessary StopIteration instances for monitoring (GH-119216) #119497

Merged
merged 2 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Doc/c-api/monitoring.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions Include/cpython/monitoring.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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
4 changes: 2 additions & 2 deletions Include/internal/pycore_opcode_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 26 additions & 5 deletions Lib/test/test_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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__):
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid creating unnecessary :exc:`StopIteration` instances for monitoring.
9 changes: 5 additions & 4 deletions Modules/_testcapi/monitoring.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
8 changes: 2 additions & 6 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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);
}
Expand Down
14 changes: 11 additions & 3 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions Python/instrumentation.c
Original file line number Diff line number Diff line change
Expand Up @@ -2622,7 +2622,7 @@ exception_event_teardown(int err, PyObject *exc) {
}
else {
assert(PyErr_Occurred());
Py_DECREF(exc);
Py_XDECREF(exc);
}
return err;
}
Expand Down Expand Up @@ -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);
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
}
Loading