fixup! Allow perf to work without frame pointers

pablogsal · pablogsal · commit 7cbd394acd21 · 2023-11-20T00:41:57.000Z
diff --git a/Lib/test/test_perf_profiler.py b/Lib/test/test_perf_profiler.py
@@ -383,6 +383,29 @@ def baz(n):
             self.assertNotIn(f"py::bar:{script}", stdout)
             self.assertNotIn(f"py::baz:{script}", stdout)
 
+@unittest.skipUnless(perf_command_works(), "perf command doesn't work")
+@unittest.skipUnless(
+    is_unwinding_reliable_with_frame_pointers(),
+    "Unwinding is unreliable with frame pointers",
+)
+class TestPerfProfiler(unittest.TestCase, TestPerfProfilerMixin):
+    def run_perf(self, script_dir, script, activate_trampoline=True):
+        if activate_trampoline:
+            return run_perf(script_dir, sys.executable, "-Xperf", script)
+        return run_perf(script_dir, sys.executable, script)
+
+    def setUp(self):
+        super().setUp()
+        self.perf_files = set(pathlib.Path("/tmp/").glob("perf-*.map"))
+
+    def tearDown(self) -> None:
+        super().tearDown()
+        files_to_delete = (
+            set(pathlib.Path("/tmp/").glob("perf-*.map")) - self.perf_files
+        )
+        for file in files_to_delete:
+            file.unlink()
+
     def test_pre_fork_compile(self):
         code = """if 1:
                 import sys
@@ -400,7 +423,7 @@ def bar_fork():
                     foo_fork()
 
                 def foo():
-                    pass
+                    import time; time.sleep(1)
 
                 def bar():
                     foo()
@@ -457,30 +480,6 @@ def compile_trampolines_for_all_functions():
                 self.assertIn(line, child_perf_file_contents)
 
 
-@unittest.skipUnless(perf_command_works(), "perf command doesn't work")
-@unittest.skipUnless(
-    is_unwinding_reliable_with_frame_pointers(),
-    "Unwinding is unreliable with frame pointers",
-)
-class TestPerfProfiler(unittest.TestCase, TestPerfProfilerMixin):
-    def run_perf(self, script_dir, script, activate_trampoline=True):
-        if activate_trampoline:
-            return run_perf(script_dir, sys.executable, "-Xperf", script)
-        return run_perf(script_dir, sys.executable, script)
-
-    def setUp(self):
-        super().setUp()
-        self.perf_files = set(pathlib.Path("/tmp/").glob("perf-*.map"))
-
-    def tearDown(self) -> None:
-        super().tearDown()
-        files_to_delete = (
-            set(pathlib.Path("/tmp/").glob("perf-*.map")) - self.perf_files
-        )
-        for file in files_to_delete:
-            file.unlink()
-
-
 @unittest.skipUnless(perf_command_works(), "perf command doesn't work")
 class TestPerfProfilerWithDwarf(unittest.TestCase, TestPerfProfilerMixin):
     def run_perf(self, script_dir, script, activate_trampoline=True):
diff --git a/Python/perf_trampoline.c b/Python/perf_trampoline.c
@@ -318,6 +318,7 @@ static int perf_map_fini(void* state) {
         // clean up the lock and state
         PyThread_free_lock(perf_map_state.map_lock);
         perf_map_state.perf_map = NULL;
+        trampoline_api.state = NULL;
     }
     return 0;
 }
@@ -655,6 +656,7 @@ static int perf_map_jit_fini(void* state) {
     if (perf_jit_map_state.mapped_buffer != NULL) {
         munmap(perf_jit_map_state.mapped_buffer, perf_jit_map_state.mapped_size);
     }
+    trampoline_api.state = NULL;
     return 0;
 }
 
@@ -807,6 +809,9 @@ py_trampoline_evaluator(PyThreadState *ts, _PyInterpreterFrame *frame,
 int PyUnstable_PerfTrampoline_CompileCode(PyCodeObject *co)
 {
 #ifdef PY_HAVE_PERF_TRAMPOLINE
+    if (perf_status != PERF_STATUS_OK) {
+        return -1;
+    }
     py_trampoline f = NULL;
     assert(extra_code_index != -1);
     int ret = _PyCode_GetExtra((PyObject *)co, extra_code_index, (void **)&f);
@@ -911,6 +916,16 @@ _PyPerfTrampoline_Init(int activate)
         perf_status = PERF_STATUS_OK;
     }
 
+    if (trampoline_api.init_state) {
+        trampoline_api.state = trampoline_api.init_state();
+        if (trampoline_api.state == NULL) {
+            return -1;
+        }
+    }
+    else {
+        trampoline_api.state = NULL;
+    }
+
 #endif
     return 0;
 }
@@ -947,15 +962,17 @@ _PyPerfTrampoline_AfterFork_Child(void)
         char filename[256];
         pid_t parent_pid = getppid();
         snprintf(filename, sizeof(filename), "/tmp/perf-%d.map", parent_pid);
-        if (PyUnstable_CopyPerfMapFile(filename) != 0) {
+        if ( PyUnstable_CopyPerfMapFile(filename) != 0) {
             return PyStatus_Error("Failed to copy perf map file.");
         }
     } else {
+        PyUnstable_PerfMapState_Fini();
         // Restart trampoline in file in child.
         int was_active = _PyIsPerfTrampolineActive();
         _PyPerfTrampoline_Fini();
         if (was_active) {
             _PyPerfTrampoline_Init(1);
+            trampoline_api.init_state();
         }
     }
 #endif
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -2379,13 +2379,10 @@ PyAPI_FUNC(void) PyUnstable_PerfMapState_Fini(void) {
 
 PyAPI_FUNC(int) PyUnstable_CopyPerfMapFile(const char* parent_filename) {
 #ifndef MS_WINDOWS
-        void *state = _PyPerfTrampoline_GetState();
-        if (state) {
-            _PyPerf_Callbacks cur_cb;
-            _PyPerfTrampoline_GetCallbacks(&cur_cb);
-            if (cur_cb.copy_file != NULL) {
-                cur_cb.copy_file(parent_filename);
-            }
+        _PyPerf_Callbacks cur_cb;
+        _PyPerfTrampoline_GetCallbacks(&cur_cb);
+        if (cur_cb.copy_file != NULL) {
+            cur_cb.copy_file(parent_filename);
         }
 #endif
     return 0;
