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

Avoid creating a StopIteration instance for monitoring #118692

Closed
scoder opened this issue May 7, 2024 · 1 comment · Fixed by #119216
Closed

Avoid creating a StopIteration instance for monitoring #118692

scoder opened this issue May 7, 2024 · 1 comment · Fixed by #119216
Assignees
Labels
3.13 bugs and security fixes type-feature A feature request or enhancement

Comments

@scoder
Copy link
Contributor

scoder commented May 7, 2024

Feature or enhancement

Proposal:

A new C-API for injecting monitoring events was discussed in #111997 and implemented for Py3.13 in #116413

A remaining issue is that signalling "STOP_ITERATION" events requires a StopIteration exception instance, whereas generators usually try to avoid creating one during termination to increase their performance.

I think it would be best if CPython's monitoring infrastructure created a StopIteration instance automatically when it detects that it needs to notify about it without having one. It's in the best position to detect whether it really needs an exception instance or not. Event sources shouldn't be forced to a) figure that out themselves or b) create an instance just for it to be thrown away.

CPython already creates an exception instance for submitting a monitoring event for a generator, but does so unconditionally. It would also benefit from not having to generate one unless it's really needed for signalling.

cpython/Python/bytecodes.c

Lines 289 to 317 in 5248596

macro(END_FOR) = POP_TOP;
tier1 inst(INSTRUMENTED_END_FOR, (receiver, value -- receiver)) {
/* 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)) {
ERROR_NO_POP();
}
PyErr_SetRaisedException(NULL);
}
DECREF_INPUTS();
}
pure inst(END_SEND, (receiver, value -- value)) {
Py_DECREF(receiver);
}
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)) {
ERROR_NO_POP();
}
PyErr_SetRaisedException(NULL);
}
Py_DECREF(receiver);
}

Basically, we should move the existing code from the byte code handlers all the way to the other side into the notification mechanism.

In order to report the correct StopIteration value, the event creation function should accept an object value directly, and create a StopIteration instance for that value only if it ends up signalling it to listeners.

We might allow passing the value NULL (as opposed to the object value None) to make the machinery look up an actually existing currently raised StopIteration instance. Alternatively, passing a StopIteration instance as value is probably also a reasonably clear indication for an instance being available already. We may also consider adding two event creation functions, one with a bare stop value and one with a readily created StopIteration instance.

This ticket proposes a usability/performance enhancement to a newly added feature in Py3.13, and thus does not fall under the "beta-1" release end of new features.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

#116413 (comment)

Linked PRs

@scoder scoder added type-feature A feature request or enhancement 3.13 bugs and security fixes labels May 7, 2024
@scoder
Copy link
Contributor Author

scoder commented May 7, 2024

@iritkatriel iritkatriel self-assigned this May 7, 2024
iritkatriel added a commit to iritkatriel/cpython that referenced this issue May 20, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 24, 2024
…or monitoring (pythonGH-119216)

(cherry picked from commit 6e9863d)

Co-authored-by: Irit Katriel <[email protected]>
iritkatriel added a commit that referenced this issue May 24, 2024
…for monitoring (GH-119216) (#119497)

* gh-118692: Avoid creating unnecessary StopIteration instances for monitoring (GH-119216)
(cherry picked from commit 6e9863d)

---------

Co-authored-by: Irit Katriel <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants