From 6e53f53d5b1891a23ab9a6d6f41658860e65c590 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sat, 3 Feb 2024 23:29:02 -0800 Subject: [PATCH 1/9] Improve performance of sys.settrace --- Python/ceval.c | 28 +++++++++++++++++----------- Python/ceval_macros.h | 16 ++++++++++------ Python/instrumentation.c | 4 +--- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 4f208009086191..c662024dd43633 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -795,17 +795,23 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int { _Py_CODEUNIT *prev = frame->instr_ptr; _Py_CODEUNIT *here = frame->instr_ptr = next_instr; - _PyFrame_SetStackPointer(frame, stack_pointer); - int original_opcode = _Py_call_instrumentation_line( - tstate, frame, here, prev); - stack_pointer = _PyFrame_GetStackPointer(frame); - if (original_opcode < 0) { - next_instr = here+1; - goto error; - } - next_instr = frame->instr_ptr; - if (next_instr != here) { - DISPATCH(); + int original_opcode = 0; + if (tstate->tracing) { + PyCodeObject *code = _PyFrame_GetCode(frame); + original_opcode = code->_co_monitoring->lines[(int)(here - _PyCode_CODE(code))].original_opcode; + } else { + _PyFrame_SetStackPointer(frame, stack_pointer); + original_opcode = _Py_call_instrumentation_line( + tstate, frame, here, prev); + stack_pointer = _PyFrame_GetStackPointer(frame); + if (original_opcode < 0) { + next_instr = here+1; + goto error; + } + next_instr = frame->instr_ptr; + if (next_instr != here) { + DISPATCH(); + } } if (_PyOpcode_Caches[original_opcode]) { _PyBinaryOpCache *cache = (_PyBinaryOpCache *)(next_instr+1); diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index c2550f53ad6eaa..5e1a6e02b71fb1 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -332,12 +332,16 @@ do { \ // for an exception handler, displaying the traceback, and so on #define INSTRUMENTED_JUMP(src, dest, event) \ do { \ - _PyFrame_SetStackPointer(frame, stack_pointer); \ - next_instr = _Py_call_instrumentation_jump(tstate, event, frame, src, dest); \ - stack_pointer = _PyFrame_GetStackPointer(frame); \ - if (next_instr == NULL) { \ - next_instr = (dest)+1; \ - goto error; \ + if (tstate->tracing) {\ + next_instr = dest; \ + } else { \ + _PyFrame_SetStackPointer(frame, stack_pointer); \ + next_instr = _Py_call_instrumentation_jump(tstate, event, frame, src, dest); \ + stack_pointer = _PyFrame_GetStackPointer(frame); \ + if (next_instr == NULL) { \ + next_instr = (dest)+1; \ + goto error; \ + } \ } \ } while (0); diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 533aece210202b..7cc2bb42ed73ef 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1130,6 +1130,7 @@ _Py_Instrumentation_GetLine(PyCodeObject *code, int index) int _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *prev) { + // The caller should guarantee tstate->tracing == 0 PyCodeObject *code = _PyFrame_GetCode(frame); assert(is_version_up_to_date(code, tstate->interp)); assert(instrumentation_cross_checks(tstate->interp, code)); @@ -1137,9 +1138,6 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, _PyCoMonitoringData *monitoring = code->_co_monitoring; _PyCoLineInstrumentationData *line_data = &monitoring->lines[i]; - if (tstate->tracing) { - goto done; - } PyInterpreterState *interp = tstate->interp; int8_t line_delta = line_data->line_delta; int line = compute_line(code, i, line_delta); From a42d7c816f706a09eb9e8cbe0ecadf0deea2445e Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 4 Feb 2024 07:45:30 +0000 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-02-04-07-45-29.gh-issue-107674.q8mCmi.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-02-04-07-45-29.gh-issue-107674.q8mCmi.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-02-04-07-45-29.gh-issue-107674.q8mCmi.rst b/Misc/NEWS.d/next/Core and Builtins/2024-02-04-07-45-29.gh-issue-107674.q8mCmi.rst new file mode 100644 index 00000000000000..f9b96788bfad94 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-02-04-07-45-29.gh-issue-107674.q8mCmi.rst @@ -0,0 +1 @@ +Improved the performance of :func:`sys.settrace` significantly From d5b1dec2a57999853cd20efe8bb3ef65e479517b Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 4 Feb 2024 23:50:41 -0800 Subject: [PATCH 3/9] Do not instrument code when tracing --- Python/bytecodes.c | 17 ++++++++++------- Python/generated_cases.c.h | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6fb4d719e43991..67fae1b8d1e15f 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -148,16 +148,19 @@ dummy_func( ~_PY_EVAL_EVENTS_MASK; uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; assert((code_version & 255) == 0); - if (code_version != global_version) { - int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); - ERROR_IF(err, error); - next_instr = this_instr; + if (tstate->tracing == 0) { + if (code_version != global_version) { + int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); + ERROR_IF(err, error); + next_instr = this_instr; + } else { + this_instr->op.code = RESUME_CHECK; + } } - else { + if (next_instr != this_instr) { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } - this_instr->op.code = RESUME_CHECK; } } @@ -175,7 +178,7 @@ dummy_func( inst(INSTRUMENTED_RESUME, (--)) { uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & ~_PY_EVAL_EVENTS_MASK; uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; - if (code_version != global_version) { + if (code_version != global_version && tstate->tracing == 0) { if (_Py_Instrument(_PyFrame_GetCode(frame), tstate->interp)) { GOTO_ERROR(error); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 16f1db30620d72..1f3a895fe104d3 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3130,7 +3130,7 @@ INSTRUCTION_STATS(INSTRUMENTED_RESUME); uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & ~_PY_EVAL_EVENTS_MASK; uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; - if (code_version != global_version) { + if (code_version != global_version && tstate->tracing == 0) { if (_Py_Instrument(_PyFrame_GetCode(frame), tstate->interp)) { GOTO_ERROR(error); } @@ -4799,16 +4799,19 @@ ~_PY_EVAL_EVENTS_MASK; uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; assert((code_version & 255) == 0); - if (code_version != global_version) { - int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); - if (err) goto error; - next_instr = this_instr; + if (tstate->tracing == 0) { + if (code_version != global_version) { + int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); + if (err) goto error; + next_instr = this_instr; + } else { + this_instr->op.code = RESUME_CHECK; + } } - else { + if (next_instr != this_instr) { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } - this_instr->op.code = RESUME_CHECK; } DISPATCH(); } From 2a1eefb392c714a842f82c6705d538ce7a1a6da0 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 4 Feb 2024 23:57:01 -0800 Subject: [PATCH 4/9] Reduce extra branch and save some operations --- Python/bytecodes.c | 16 +++++++++------- Python/generated_cases.c.h | 16 +++++++++------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 67fae1b8d1e15f..f7b5e7fd81a398 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -143,21 +143,23 @@ dummy_func( inst(RESUME, (--)) { TIER_ONE_ONLY assert(frame == tstate->current_frame); - uintptr_t global_version = - _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & - ~_PY_EVAL_EVENTS_MASK; - uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; - assert((code_version & 255) == 0); if (tstate->tracing == 0) { + uintptr_t global_version = + _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & + ~_PY_EVAL_EVENTS_MASK; + uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; + assert((code_version & 255) == 0); if (code_version != global_version) { int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); ERROR_IF(err, error); next_instr = this_instr; } else { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + CHECK_EVAL_BREAKER(); + } this_instr->op.code = RESUME_CHECK; } - } - if (next_instr != this_instr) { + } else { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 1f3a895fe104d3..d125340540bfda 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4794,21 +4794,23 @@ _Py_CODEUNIT *this_instr = next_instr - 1; TIER_ONE_ONLY assert(frame == tstate->current_frame); - uintptr_t global_version = - _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & - ~_PY_EVAL_EVENTS_MASK; - uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; - assert((code_version & 255) == 0); if (tstate->tracing == 0) { + uintptr_t global_version = + _Py_atomic_load_uintptr_relaxed(&tstate->interp->ceval.eval_breaker) & + ~_PY_EVAL_EVENTS_MASK; + uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version; + assert((code_version & 255) == 0); if (code_version != global_version) { int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); if (err) goto error; next_instr = this_instr; } else { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + CHECK_EVAL_BREAKER(); + } this_instr->op.code = RESUME_CHECK; } - } - if (next_instr != this_instr) { + } else { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } From cb09c5575874e724042ed825b6b1be5c943caa24 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Mon, 5 Feb 2024 10:58:45 -0800 Subject: [PATCH 5/9] Change the comment to assertion --- Python/instrumentation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 7cc2bb42ed73ef..9b009b4cffe87d 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1130,8 +1130,8 @@ _Py_Instrumentation_GetLine(PyCodeObject *code, int index) int _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *prev) { - // The caller should guarantee tstate->tracing == 0 PyCodeObject *code = _PyFrame_GetCode(frame); + assert(tstate->tracing == 0); assert(is_version_up_to_date(code, tstate->interp)); assert(instrumentation_cross_checks(tstate->interp, code)); int i = (int)(instr - _PyCode_CODE(code)); From 74d337516213d49d130e3462450f1e99f3875846 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 25 Feb 2024 21:10:19 -0800 Subject: [PATCH 6/9] Add missing instr change --- Python/bytecodes.c | 2 ++ Python/generated_cases.c.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 1106c7afcd004c..b186b5e5398a81 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -158,11 +158,13 @@ dummy_func( if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } + this_instr->op.code = RESUME_CHECK; } } else { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } + this_instr->op.code = RESUME_CHECK; } } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index e0a9c4d0b71de0..5141234f6c8e70 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4795,11 +4795,13 @@ if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } + this_instr->op.code = RESUME_CHECK; } } else { if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } + this_instr->op.code = RESUME_CHECK; } DISPATCH(); } From 47ff5548074e4dfe72c47b3e2f6a20c66ee5caaf Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Mon, 26 Feb 2024 11:50:16 -0800 Subject: [PATCH 7/9] Do not deopt RESUME_CHECK if it's tracing --- Python/bytecodes.c | 2 +- Python/generated_cases.c.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b186b5e5398a81..81b8ee1a61890e 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -176,7 +176,7 @@ dummy_func( uintptr_t eval_breaker = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker); uintptr_t version = _PyFrame_GetCode(frame)->_co_instrumentation_version; assert((version & _PY_EVAL_EVENTS_MASK) == 0); - DEOPT_IF(eval_breaker != version); + DEOPT_IF(eval_breaker != version && tstate->tracing == 0); } inst(INSTRUMENTED_RESUME, (--)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 5141234f6c8e70..de9fbd8ee1cad4 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4818,7 +4818,7 @@ uintptr_t eval_breaker = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker); uintptr_t version = _PyFrame_GetCode(frame)->_co_instrumentation_version; assert((version & _PY_EVAL_EVENTS_MASK) == 0); - DEOPT_IF(eval_breaker != version, RESUME); + DEOPT_IF(eval_breaker != version && tstate->tracing == 0, RESUME); DISPATCH(); } From 79f43b93f37ffbfebc80fdf2753b21ad8f419824 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Mon, 26 Feb 2024 13:28:43 -0800 Subject: [PATCH 8/9] Regen executor_cases --- Python/executor_cases.c.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 0b1d75985e1195..20fab8f4c61eb5 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -20,7 +20,7 @@ uintptr_t eval_breaker = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker); uintptr_t version = _PyFrame_GetCode(frame)->_co_instrumentation_version; assert((version & _PY_EVAL_EVENTS_MASK) == 0); - if (eval_breaker != version) goto deoptimize; + if (eval_breaker != version && tstate->tracing == 0) goto deoptimize; break; } From 0c72adae3fd8e03323fb20126989d08ddeca98d9 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Tue, 27 Feb 2024 10:52:15 -0800 Subject: [PATCH 9/9] Clean up bytecode code --- Python/bytecodes.c | 16 +++++----------- Python/generated_cases.c.h | 16 +++++----------- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 81b8ee1a61890e..565379afc4b5a7 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -153,19 +153,13 @@ dummy_func( int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); ERROR_IF(err, error); next_instr = this_instr; + DISPATCH(); } - else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { - CHECK_EVAL_BREAKER(); - } - this_instr->op.code = RESUME_CHECK; - } - } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { - CHECK_EVAL_BREAKER(); - } - this_instr->op.code = RESUME_CHECK; } + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + CHECK_EVAL_BREAKER(); + } + this_instr->op.code = RESUME_CHECK; } inst(RESUME_CHECK, (--)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index de9fbd8ee1cad4..bb26dac0e2be20 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4790,19 +4790,13 @@ int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp); if (err) goto error; next_instr = this_instr; + DISPATCH(); } - else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { - CHECK_EVAL_BREAKER(); - } - this_instr->op.code = RESUME_CHECK; - } - } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { - CHECK_EVAL_BREAKER(); - } - this_instr->op.code = RESUME_CHECK; } + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + CHECK_EVAL_BREAKER(); + } + this_instr->op.code = RESUME_CHECK; DISPATCH(); }