From 0ccd3bbd2d7143036e52c7f8fb08875ace58263f Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Sat, 5 Aug 2023 01:57:54 +0100 Subject: [PATCH 1/6] Use PyLong_FromLong instead of PyLong_FromSsize_t in instrumentation.c. --- Python/instrumentation.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 123c20dfe1a99b..df5c3a33d71dcb 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -961,7 +961,7 @@ call_instrumentation_vector( /* Offset visible to user should be the offset in bytes, as that is the * convention for APIs involving code offsets. */ int bytes_offset = offset * (int)sizeof(_Py_CODEUNIT); - PyObject *offset_obj = PyLong_FromSsize_t(bytes_offset); + PyObject *offset_obj = PyLong_FromLong(bytes_offset); if (offset_obj == NULL) { return -1; } @@ -1151,7 +1151,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, (interp->monitors.tools[PY_MONITORING_EVENT_LINE] | code->_co_monitoring->local_monitors.tools[PY_MONITORING_EVENT_LINE] ); - PyObject *line_obj = PyLong_FromSsize_t(line); + PyObject *line_obj = PyLong_FromLong(line); if (line_obj == NULL) { return -1; } @@ -1205,7 +1205,7 @@ _Py_call_instrumentation_instruction(PyThreadState *tstate, _PyInterpreterFrame* code->_co_monitoring->local_monitors.tools[PY_MONITORING_EVENT_INSTRUCTION] ); int bytes_offset = offset * (int)sizeof(_Py_CODEUNIT); - PyObject *offset_obj = PyLong_FromSsize_t(bytes_offset); + PyObject *offset_obj = PyLong_FromLong(bytes_offset); if (offset_obj == NULL) { return -1; } From 83ffca904ca888038303bdd64a31105853d22fd7 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Sat, 5 Aug 2023 03:01:07 +0100 Subject: [PATCH 2/6] Avoid boxing and unboxing line number for sys.settrace --- Python/instrumentation.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index df5c3a33d71dcb..62228ca3b46106 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1151,6 +1151,32 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, (interp->monitors.tools[PY_MONITORING_EVENT_LINE] | code->_co_monitoring->local_monitors.tools[PY_MONITORING_EVENT_LINE] ); + /* Special case sys.settrace to avoid boxing the line number, + * only to unbox it. */ + if (tools & (1 << PY_MONITORING_SYS_TRACE_ID)) { + if (tstate->c_tracefunc != NULL && line >= 0) { + PyFrameObject *frame_obj = _PyFrame_GetFrameObject(frame); + if (frame_obj == NULL) { + return -1; + } + if (frame_obj->f_trace_lines) { + int old_what = tstate->what_event; + tstate->what_event = PY_MONITORING_EVENT_LINE; + tstate->tracing++; + Py_INCREF(frame_obj); + frame_obj->f_lineno = line; + int err = tstate->c_tracefunc(tstate->c_traceobj, frame_obj, PyTrace_LINE, Py_None); + frame_obj->f_lineno = 0; + tstate->tracing--; + tstate->what_event = old_what; + Py_DECREF(frame_obj); + if (err) { + return -1; + } + } + } + tools &= (255 - (1 << PY_MONITORING_SYS_TRACE_ID)); + } PyObject *line_obj = PyLong_FromLong(line); if (line_obj == NULL) { return -1; @@ -1158,7 +1184,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, PyObject *args[3] = { NULL, (PyObject *)code, line_obj }; while (tools) { int tool = most_significant_bit(tools); - assert(tool >= 0 && tool < 8); + assert(tool >= 0 && tool < PY_MONITORING_SYS_PROFILE_ID); assert(tools & (1 << tool)); tools &= ~(1 << tool); int res = call_one_instrument(interp, tstate, &args[1], From de050f2c3aa38d7b4b1a922b020f8d45370cf34e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Sat, 5 Aug 2023 04:47:32 +0100 Subject: [PATCH 3/6] Add news --- .../2023-08-05-04-47-18.gh-issue-107674.0sYhR2.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-08-05-04-47-18.gh-issue-107674.0sYhR2.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-05-04-47-18.gh-issue-107674.0sYhR2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-05-04-47-18.gh-issue-107674.0sYhR2.rst new file mode 100644 index 00000000000000..acfbf1fa2adf2c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-05-04-47-18.gh-issue-107674.0sYhR2.rst @@ -0,0 +1 @@ +Fixed performance regression in ``sys.settrace``. From 9077b17598633be15ebc8cff3858100975338c51 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 9 Aug 2023 19:38:05 +0100 Subject: [PATCH 4/6] Address review comments --- Python/instrumentation.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 62228ca3b46106..c47d09cf12927f 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1152,7 +1152,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, code->_co_monitoring->local_monitors.tools[PY_MONITORING_EVENT_LINE] ); /* Special case sys.settrace to avoid boxing the line number, - * only to unbox it. */ + * only to immediately unbox it. */ if (tools & (1 << PY_MONITORING_SYS_TRACE_ID)) { if (tstate->c_tracefunc != NULL && line >= 0) { PyFrameObject *frame_obj = _PyFrame_GetFrameObject(frame); @@ -1160,9 +1160,12 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, return -1; } if (frame_obj->f_trace_lines) { + /* Set tracing and what_event as we do for + * instrumentation */ int old_what = tstate->what_event; tstate->what_event = PY_MONITORING_EVENT_LINE; tstate->tracing++; + /* Call c_tracefunc directly */ Py_INCREF(frame_obj); frame_obj->f_lineno = line; int err = tstate->c_tracefunc(tstate->c_traceobj, frame_obj, PyTrace_LINE, Py_None); @@ -1177,12 +1180,15 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, } tools &= (255 - (1 << PY_MONITORING_SYS_TRACE_ID)); } + if (tools == 0) { + goto done; + } PyObject *line_obj = PyLong_FromLong(line); if (line_obj == NULL) { return -1; } PyObject *args[3] = { NULL, (PyObject *)code, line_obj }; - while (tools) { + do { int tool = most_significant_bit(tools); assert(tool >= 0 && tool < PY_MONITORING_SYS_PROFILE_ID); assert(tools & (1 << tool)); @@ -1202,7 +1208,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, /* DISABLE */ remove_line_tools(code, i, 1 << tool); } - } + } while (tools); Py_DECREF(line_obj); done: assert(original_opcode != 0); From 7460a99ecdb4881b1771e5ab3a2e258d630700c8 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 9 Aug 2023 19:59:32 +0100 Subject: [PATCH 5/6] Clarify comments --- Python/instrumentation.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index c47d09cf12927f..92ca6b737c1c0d 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1158,14 +1158,14 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, PyFrameObject *frame_obj = _PyFrame_GetFrameObject(frame); if (frame_obj == NULL) { return -1; - } +// } if (frame_obj->f_trace_lines) { - /* Set tracing and what_event as we do for - * instrumentation */ + /* Need to set tracing and what_event as if using + * the instrumentation call. */ int old_what = tstate->what_event; tstate->what_event = PY_MONITORING_EVENT_LINE; tstate->tracing++; - /* Call c_tracefunc directly */ + /* Call c_tracefunc directly, having set the line number. */ Py_INCREF(frame_obj); frame_obj->f_lineno = line; int err = tstate->c_tracefunc(tstate->c_traceobj, frame_obj, PyTrace_LINE, Py_None); From 6b4bafbf422e3ab34d947603b9370a489e0ae76c Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 9 Aug 2023 22:22:07 +0100 Subject: [PATCH 6/6] Fix editing --- Python/instrumentation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 92ca6b737c1c0d..c4321068010506 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1158,7 +1158,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, PyFrameObject *frame_obj = _PyFrame_GetFrameObject(frame); if (frame_obj == NULL) { return -1; -// } + } if (frame_obj->f_trace_lines) { /* Need to set tracing and what_event as if using * the instrumentation call. */