gh-107674: Lazy load line number to improve performance of tracing (GH-118127)

gaogaotiantian · web-flow · commit 375c94c75dd9 · 2024-04-29T09:54:52.000+01:00
diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-04-20-20-30-15.gh-issue-107674.GZPOP7.rst b/Misc/NEWS.d/next/Core and Builtins/2024-04-20-20-30-15.gh-issue-107674.GZPOP7.rst
@@ -0,0 +1 @@
+Lazy load frame line number to improve performance of tracing
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
@@ -41,12 +41,20 @@ int
 PyFrame_GetLineNumber(PyFrameObject *f)
 {
     assert(f != NULL);
-    if (f->f_lineno != 0) {
-        return f->f_lineno;
+    if (f->f_lineno == -1) {
+        // We should calculate it once. If we can't get the line number,
+        // set f->f_lineno to 0.
+        f->f_lineno = PyUnstable_InterpreterFrame_GetLine(f->f_frame);
+        if (f->f_lineno < 0) {
+            f->f_lineno = 0;
+            return -1;
+        }
     }
-    else {
-        return PyUnstable_InterpreterFrame_GetLine(f->f_frame);
+
+    if (f->f_lineno > 0) {
+        return f->f_lineno;
     }
+    return PyUnstable_InterpreterFrame_GetLine(f->f_frame);
 }
 
 static PyObject *
diff --git a/Python/instrumentation.c b/Python/instrumentation.c
@@ -268,14 +268,15 @@ get_events(_Py_GlobalMonitors *m, int tool_id)
  * 8 bit value.
  * if line_delta == -128:
  *     line = None # represented as -1
- * elif line_delta == -127:
+ * elif line_delta == -127 or line_delta == -126:
  *     line = PyCode_Addr2Line(code, offset * sizeof(_Py_CODEUNIT));
  * else:
  *     line = first_line  + (offset >> OFFSET_SHIFT) + line_delta;
  */
 
 #define NO_LINE -128
-#define COMPUTED_LINE -127
+#define COMPUTED_LINE_LINENO_CHANGE -127
+#define COMPUTED_LINE -126
 
 #define OFFSET_SHIFT 4
 
@@ -302,7 +303,7 @@ compute_line(PyCodeObject *code, int offset, int8_t line_delta)
 
         return -1;
     }
-    assert(line_delta == COMPUTED_LINE);
+    assert(line_delta == COMPUTED_LINE || line_delta == COMPUTED_LINE_LINENO_CHANGE);
     /* Look it up */
     return PyCode_Addr2Line(code, offset * sizeof(_Py_CODEUNIT));
 }
@@ -1224,18 +1225,26 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
     }
     PyInterpreterState *interp = tstate->interp;
     int8_t line_delta = line_data->line_delta;
-    int line = compute_line(code, i, line_delta);
-    assert(line >= 0);
-    assert(prev != NULL);
-    int prev_index = (int)(prev - _PyCode_CODE(code));
-    int prev_line = _Py_Instrumentation_GetLine(code, prev_index);
-    if (prev_line == line) {
-        int prev_opcode = _PyCode_CODE(code)[prev_index].op.code;
-        /* RESUME and INSTRUMENTED_RESUME are needed for the operation of
-         * instrumentation, so must never be hidden by an INSTRUMENTED_LINE.
-         */
-        if (prev_opcode != RESUME && prev_opcode != INSTRUMENTED_RESUME) {
-            goto done;
+    int line = 0;
+
+    if (line_delta == COMPUTED_LINE_LINENO_CHANGE) {
+        // We know the line number must have changed, don't need to calculate
+        // the line number for now because we might not need it.
+        line = -1;
+    } else {
+        line = compute_line(code, i, line_delta);
+        assert(line >= 0);
+        assert(prev != NULL);
+        int prev_index = (int)(prev - _PyCode_CODE(code));
+        int prev_line = _Py_Instrumentation_GetLine(code, prev_index);
+        if (prev_line == line) {
+            int prev_opcode = _PyCode_CODE(code)[prev_index].op.code;
+            /* RESUME and INSTRUMENTED_RESUME are needed for the operation of
+             * instrumentation, so must never be hidden by an INSTRUMENTED_LINE.
+             */
+            if (prev_opcode != RESUME && prev_opcode != INSTRUMENTED_RESUME) {
+                goto done;
+            }
         }
     }
 
@@ -1260,6 +1269,12 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
                 tstate->tracing++;
                 /* Call c_tracefunc directly, having set the line number. */
                 Py_INCREF(frame_obj);
+                if (line == -1 && line_delta > COMPUTED_LINE) {
+                    /* Only assign f_lineno if it's easy to calculate, otherwise
+                     * do lazy calculation by setting the f_lineno to 0.
+                     */
+                    line = compute_line(code, i, line_delta);
+                }
                 frame_obj->f_lineno = line;
                 int err = tstate->c_tracefunc(tstate->c_traceobj, frame_obj, PyTrace_LINE, Py_None);
                 frame_obj->f_lineno = 0;
@@ -1276,6 +1291,11 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
     if (tools == 0) {
         goto done;
     }
+
+    if (line == -1) {
+        /* Need to calculate the line number now for monitoring events */
+        line = compute_line(code, i, line_delta);
+    }
     PyObject *line_obj = PyLong_FromLong(line);
     if (line_obj == NULL) {
         return -1;
@@ -1477,6 +1497,13 @@ initialize_lines(PyCodeObject *code)
                  */
                 if (line != current_line && line >= 0) {
                     line_data[i].original_opcode = opcode;
+                    if (line_data[i].line_delta == COMPUTED_LINE) {
+                        /* Label this line as a line with a line number change
+                         * which could help the monitoring callback to quickly
+                         * identify the line number change.
+                         */
+                        line_data[i].line_delta = COMPUTED_LINE_LINENO_CHANGE;
+                    }
                 }
                 else {
                     line_data[i].original_opcode = 0;
@@ -1529,6 +1556,11 @@ initialize_lines(PyCodeObject *code)
         assert(target >= 0);
         if (line_data[target].line_delta != NO_LINE) {
             line_data[target].original_opcode = _Py_GetBaseOpcode(code, target);
+            if (line_data[target].line_delta == COMPUTED_LINE_LINENO_CHANGE) {
+                // If the line is a jump target, we are not sure if the line
+                // number changes, so we set it to COMPUTED_LINE.
+                line_data[target].line_delta = COMPUTED_LINE;
+            }
         }
     }
     /* Scan exception table */
diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c
@@ -174,6 +174,7 @@ call_trace_func(_PyLegacyEventHandler *self, PyObject *arg)
 
     Py_INCREF(frame);
     int err = tstate->c_tracefunc(tstate->c_traceobj, frame, self->event, arg);
+    frame->f_lineno = 0;
     Py_DECREF(frame);
     if (err) {
         return NULL;

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Lazy load frame line number to improve performance of tracing`
Original file line number	Diff line number	Diff line change
`@@ -41,12 +41,20 @@ int`
`41`	`41`	`PyFrame_GetLineNumber(PyFrameObject *f)`
`42`	`42`	`{`
`43`	`43`	`assert(f != NULL);`
`44`		`- if (f->f_lineno != 0) {`
`45`		`- return f->f_lineno;`
	`44`	`+ if (f->f_lineno == -1) {`
	`45`	`+ // We should calculate it once. If we can't get the line number,`
	`46`	`+ // set f->f_lineno to 0.`
	`47`	`+ f->f_lineno = PyUnstable_InterpreterFrame_GetLine(f->f_frame);`
	`48`	`+ if (f->f_lineno < 0) {`
	`49`	`+ f->f_lineno = 0;`
	`50`	`+ return -1;`
	`51`	`+ }`
`46`	`52`	`}`
`47`		`- else {`
`48`		`- return PyUnstable_InterpreterFrame_GetLine(f->f_frame);`
	`53`	`+`
	`54`	`+ if (f->f_lineno > 0) {`
	`55`	`+ return f->f_lineno;`
`49`	`56`	`}`
	`57`	`+ return PyUnstable_InterpreterFrame_GetLine(f->f_frame);`
`50`	`58`	`}`
`51`	`59`
`52`	`60`	`static PyObject *`