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

GH-113486: Do not emit spurious PY_UNWIND events for optimized calls to classes. #113680

Merged
merged 2 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions Include/cpython/code.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ struct PyCodeObject _PyCode_DEF(1);
#define CO_FUTURE_GENERATOR_STOP 0x800000
#define CO_FUTURE_ANNOTATIONS 0x1000000

#define CO_NO_MONITORING_EVENTS 0x2000000

/* This should be defined if a future statement modifies the syntax.
For example, when a keyword is added.
*/
Expand Down
78 changes: 55 additions & 23 deletions Lib/test/test_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ class UnwindRecorder(ExceptionRecorder):
event_type = E.PY_UNWIND

def __call__(self, code, offset, exc):
self.events.append(("unwind", type(exc)))
self.events.append(("unwind", type(exc), code.co_name))

class ExceptionHandledRecorder(ExceptionRecorder):

Expand All @@ -766,8 +766,27 @@ class ThrowRecorder(ExceptionRecorder):
def __call__(self, code, offset, exc):
self.events.append(("throw", type(exc)))

class ExceptionMonitoringTest(CheckEvents):
class CallRecorder:

event_type = E.CALL

def __init__(self, events):
self.events = events

def __call__(self, code, offset, func, arg):
self.events.append(("call", func.__name__, arg))

class ReturnRecorder:

event_type = E.PY_RETURN

def __init__(self, events):
self.events = events

def __call__(self, code, offset, val):
self.events.append(("return", code.co_name, val))

class ExceptionMonitoringTest(CheckEvents):

exception_recorders = (
ExceptionRecorder,
Expand Down Expand Up @@ -936,26 +955,48 @@ def func():
)
self.assertEqual(events[0], ("throw", IndexError))

class LineRecorder:
def test_no_unwind_for_shim_frame(self):

event_type = E.LINE
class B:
def __init__(self):
raise ValueError()

def f():
try:
return B()
except ValueError:
pass

for _ in range(100):
f()
recorders = (
ReturnRecorder,
UnwindRecorder
)
events = self.get_events(f, TEST_TOOL, recorders)
adaptive_insts = dis.get_instructions(f, adaptive=True)
self.assertIn(
"CALL_ALLOC_AND_ENTER_INIT",
[i.opname for i in adaptive_insts]
)
#There should be only one unwind event
expected = [
('unwind', ValueError, '__init__'),
('return', 'f', None),
]

def __init__(self, events):
self.events = events
self.assertEqual(events, expected)

def __call__(self, code, line):
self.events.append(("line", code.co_name, line - code.co_firstlineno))
class LineRecorder:

class CallRecorder:
event_type = E.LINE

event_type = E.CALL

def __init__(self, events):
self.events = events

def __call__(self, code, offset, func, arg):
self.events.append(("call", func.__name__, arg))
def __call__(self, code, line):
self.events.append(("line", code.co_name, line - code.co_firstlineno))

class CEventRecorder:

Expand Down Expand Up @@ -1351,15 +1392,6 @@ class BranchRecorder(JumpRecorder):
event_type = E.BRANCH
name = "branch"

class ReturnRecorder:

event_type = E.PY_RETURN

def __init__(self, events):
self.events = events

def __call__(self, code, offset, val):
self.events.append(("return", val))


JUMP_AND_BRANCH_RECORDERS = JumpRecorder, BranchRecorder
Expand Down Expand Up @@ -1449,11 +1481,11 @@ def func():
('branch', 'func', 4, 4),
('line', 'func', 5),
('line', 'meth', 1),
('return', None),
('return', 'meth', None),
('jump', 'func', 5, 5),
('jump', 'func', 5, '[offset=114]'),
('branch', 'func', '[offset=120]', '[offset=124]'),
('return', None),
('return', 'func', None),
('line', 'get_events', 11)])

class TestLoadSuperAttr(CheckEvents):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
No longer issue spurious ``PY_UNWIND`` events for optimized calls to classes.
3 changes: 3 additions & 0 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,9 @@ do_monitor_exc(PyThreadState *tstate, _PyInterpreterFrame *frame,
_Py_CODEUNIT *instr, int event)
{
assert(event < _PY_MONITORING_UNGROUPED_EVENTS);
if (_PyFrame_GetCode(frame)->co_flags & CO_NO_MONITORING_EVENTS) {
return 0;
}
PyObject *exc = PyErr_GetRaisedException();
assert(exc != NULL);
int err = _Py_call_instrumentation_arg(tstate, event, frame, instr, exc);
Expand Down
6 changes: 2 additions & 4 deletions Python/instrumentation.c
Original file line number Diff line number Diff line change
Expand Up @@ -1576,13 +1576,11 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)
}
_Py_Executors_InvalidateDependency(interp, code);
int code_len = (int)Py_SIZE(code);
/* code->_co_firsttraceable >= code_len indicates
* that no instrumentation can be inserted.
* Exit early to avoid creating instrumentation
/* Exit early to avoid creating instrumentation
* data for potential statically allocated code
* objects.
* See https://github.com/python/cpython/issues/108390 */
if (code->_co_firsttraceable >= code_len) {
if (code->co_flags & CO_NO_MONITORING_EVENTS) {
return 0;
}
if (update_instrumentation_data(code, interp)) {
Expand Down
2 changes: 1 addition & 1 deletion Python/specialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -2534,7 +2534,7 @@ const struct _PyCode_DEF(8) _Py_InitCleanup = {
.co_consts = (PyObject *)&_Py_SINGLETON(tuple_empty),
.co_names = (PyObject *)&_Py_SINGLETON(tuple_empty),
.co_exceptiontable = (PyObject *)&_Py_SINGLETON(bytes_empty),
.co_flags = CO_OPTIMIZED,
.co_flags = CO_OPTIMIZED | CO_NO_MONITORING_EVENTS,
.co_localsplusnames = (PyObject *)&_Py_SINGLETON(tuple_empty),
.co_localspluskinds = (PyObject *)&_Py_SINGLETON(bytes_empty),
.co_filename = &_Py_ID(__init__),
markshannon marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Loading