gh-135953: Simplify GC markers in the tachyon profiler (#141666)

diff --git a/Lib/profiling/sampling/gecko_collector.py b/Lib/profiling/sampling/gecko_collector.py
index 6c6700f113083e3b98325541b17eac4322e5efb9..21c427b7c862a44b309da8401ad71bec020bd753 100644 (file)
--- a/Lib/profiling/sampling/gecko_collector.py
+++ b/Lib/profiling/sampling/gecko_collector.py
@@ -141,7 +141,6 @@ class GeckoCollector(Collector):
             for thread_info in interpreter_info.threads:
                 frames = thread_info.frame_info
                 tid = thread_info.thread_id
-                gc_collecting = thread_info.gc_collecting
 
                 # Initialize thread if needed
                 if tid not in self.threads:
@@ -197,16 +196,16 @@ class GeckoCollector(Collector):
                     self._add_marker(tid, "Waiting for GIL", self.gil_wait_start.pop(tid),
                                    current_time, CATEGORY_GIL)
 
-                # Track GC events - attribute to all threads that hold the GIL during GC
-                # (GC is interpreter-wide but runs on whichever thread(s) have the GIL)
-                # If GIL switches during GC, multiple threads will get GC markers
-                if gc_collecting and has_gil:
-                    # Start GC marker if not already started for this thread
+                # Track GC events by detecting <GC> frames in the stack trace
+                # This leverages the improved GC frame tracking from commit 336366fd7ca
+                # which precisely identifies the thread that initiated GC collection
+                has_gc_frame = any(frame[2] == "<GC>" for frame in frames)
+                if has_gc_frame:
+                    # This thread initiated GC collection
                     if tid not in self.gc_start_per_thread:
                         self.gc_start_per_thread[tid] = current_time
                 elif tid in self.gc_start_per_thread:
-                    # End GC marker if it was running for this thread
-                    # (either GC finished or thread lost GIL)
+                    # End GC marker when no more GC frames are detected
                     self._add_marker(tid, "GC Collecting", self.gc_start_per_thread.pop(tid),
                                    current_time, CATEGORY_GC)
 

diff --git a/Lib/test/test_profiling/test_sampling_profiler.py b/Lib/test/test_profiling/test_sampling_profiler.py
index a24dbb55cd7babd4cbffb485059a35d03a84aa6a..2d00173c22c419fccdda3bea56202a5309d1695b 100644 (file)
--- a/Lib/test/test_profiling/test_sampling_profiler.py
+++ b/Lib/test/test_profiling/test_sampling_profiler.py
@@ -63,14 +63,13 @@ class MockFrameInfo:
 class MockThreadInfo:
     """Mock ThreadInfo for testing since the real one isn't accessible."""
 
-    def __init__(self, thread_id, frame_info, status=0, gc_collecting=False):  # Default to THREAD_STATE_RUNNING (0)
+    def __init__(self, thread_id, frame_info, status=0):  # Default to THREAD_STATE_RUNNING (0)
         self.thread_id = thread_id
         self.frame_info = frame_info
         self.status = status
-        self.gc_collecting = gc_collecting
 
     def __repr__(self):
-        return f"MockThreadInfo(thread_id={self.thread_id}, frame_info={self.frame_info}, status={self.status}, gc_collecting={self.gc_collecting})"
+        return f"MockThreadInfo(thread_id={self.thread_id}, frame_info={self.frame_info}, status={self.status})"
 
 
 class MockInterpreterInfo:
@@ -2742,7 +2741,6 @@ class TestCpuModeFiltering(unittest.TestCase):
                 self.thread_id = thread_id
                 self.frame_info = frame_info
                 self.status = status
-                self.gc_collecting = False
 
         # Create test data: active thread (HAS_GIL | ON_CPU), idle thread (neither), and another active thread
         ACTIVE_STATUS = THREAD_STATUS_HAS_GIL | THREAD_STATUS_ON_CPU  # Has GIL and on CPU

diff --git a/Modules/_remote_debugging_module.c b/Modules/_remote_debugging_module.c
index 51b3c6bac02b54bd37fd6b64a589ca8adca46095..6544e3a0ce68762fe7f22001f1ba32e1171be9dd 100644 (file)
--- a/Modules/_remote_debugging_module.c
+++ b/Modules/_remote_debugging_module.c
@@ -186,7 +186,6 @@ static PyStructSequence_Field ThreadInfo_fields[] = {
     {"thread_id", "Thread ID"},
     {"status", "Thread status (flags: HAS_GIL, ON_CPU, UNKNOWN or legacy enum)"},
     {"frame_info", "Frame information"},
-    {"gc_collecting", "Whether GC is collecting (interpreter-level)"},
     {NULL}
 };
 
@@ -2726,8 +2725,6 @@ unwind_stack_for_thread(
         goto error;
     }
 
-    int gc_collecting = GET_MEMBER(int, gc_state, unwinder->debug_offsets.gc.collecting);
-
     // Calculate thread status using flags (always)
     int status_flags = 0;
 
@@ -2827,18 +2824,10 @@ unwind_stack_for_thread(
         goto error;
     }
 
-    PyObject *py_gc_collecting = PyBool_FromLong(gc_collecting);
-    if (py_gc_collecting == NULL) {
-        set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to create gc_collecting");
-        Py_DECREF(py_status);
-        goto error;
-    }
-
     // py_status contains status flags (bitfield)
     PyStructSequence_SetItem(result, 0, thread_id);
     PyStructSequence_SetItem(result, 1, py_status);  // Steals reference
     PyStructSequence_SetItem(result, 2, frame_info); // Steals reference
-    PyStructSequence_SetItem(result, 3, py_gc_collecting); // Steals reference
 
     cleanup_stack_chunks(&chunks);
     return result;