Skip to content

Commit a29ecb1

Browse files
pablogsalpablogsal
committed
fixup! gh-135953: Add GIL contention markers to sampling profiler Gecko format
1 parent 7b2719e commit a29ecb1

File tree

2 files changed

+71
-104
lines changed

2 files changed

+71
-104
lines changed

‎Lib/profiling/sampling/gecko_collector.py‎

Lines changed: 70 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,31 @@ def __init__(self, *, skip_idle=False):
8787
# GC event tracking: track if we're currently in a GC
8888
self.potential_gc_start = None
8989

90+
def _track_state_transition(self, tid, condition, active_dict, inactive_dict,
91+
active_name, inactive_name, category, current_time):
92+
"""Track binary state transitions and emit markers.
93+
94+
Args:
95+
tid: Thread ID
96+
condition: Whether the active state is true
97+
active_dict: Dict tracking start time of active state
98+
inactive_dict: Dict tracking start time of inactive state
99+
active_name: Name for active state marker
100+
inactive_name: Name for inactive state marker
101+
category: Gecko category for the markers
102+
current_time: Current timestamp
103+
"""
104+
if condition:
105+
active_dict.setdefault(tid, current_time)
106+
if tid in inactive_dict:
107+
self._add_marker(tid, inactive_name, inactive_dict.pop(tid),
108+
current_time, category)
109+
else:
110+
inactive_dict.setdefault(tid, current_time)
111+
if tid in active_dict:
112+
self._add_marker(tid, active_name, active_dict.pop(tid),
113+
current_time, category)
114+
90115
def collect(self, stack_frames):
91116
"""Collect a sample from stack frames."""
92117
current_time = (time.time() * 1000) - self.start_time
@@ -120,83 +145,46 @@ def collect(self, stack_frames):
120145
status_flags = thread_info.status
121146
has_gil = bool(status_flags & THREAD_STATUS_HAS_GIL)
122147
on_cpu = bool(status_flags & THREAD_STATUS_ON_CPU)
123-
unknown = bool(status_flags & THREAD_STATUS_UNKNOWN)
124148
gil_requested = bool(status_flags & THREAD_STATUS_GIL_REQUESTED)
125149

126-
# Track GIL possession (interval marker: "Has GIL" / "No GIL")
127-
if has_gil:
128-
if tid not in self.has_gil_start:
129-
self.has_gil_start[tid] = current_time
130-
# End "No GIL" if it was running
131-
if tid in self.no_gil_start:
132-
start_time = self.no_gil_start[tid]
133-
self._add_marker(tid, "No GIL", start_time, current_time, CATEGORY_GIL)
134-
del self.no_gil_start[tid]
135-
else:
136-
if tid not in self.no_gil_start:
137-
self.no_gil_start[tid] = current_time
138-
# End "Has GIL" if it was running
139-
if tid in self.has_gil_start:
140-
start_time = self.has_gil_start[tid]
141-
self._add_marker(tid, "Has GIL", start_time, current_time, CATEGORY_GIL)
142-
del self.has_gil_start[tid]
143-
144-
# Track "On CPU" / "Off CPU" state
145-
if on_cpu:
146-
if tid not in self.on_cpu_start:
147-
self.on_cpu_start[tid] = current_time
148-
# End "Off CPU" if it was running
149-
if tid in self.off_cpu_start:
150-
start_time = self.off_cpu_start[tid]
151-
self._add_marker(tid, "Off CPU", start_time, current_time, CATEGORY_CPU)
152-
del self.off_cpu_start[tid]
153-
else:
154-
if tid not in self.off_cpu_start:
155-
self.off_cpu_start[tid] = current_time
156-
# End "On CPU" if it was running
157-
if tid in self.on_cpu_start:
158-
start_time = self.on_cpu_start[tid]
159-
self._add_marker(tid, "On CPU", start_time, current_time, CATEGORY_CPU)
160-
del self.on_cpu_start[tid]
161-
162-
# Track "Python Code" (has GIL) / "Native Code" (on CPU without GIL)
150+
# Track GIL possession (Has GIL / No GIL)
151+
self._track_state_transition(
152+
tid, has_gil, self.has_gil_start, self.no_gil_start,
153+
"Has GIL", "No GIL", CATEGORY_GIL, current_time
154+
)
155+
156+
# Track CPU state (On CPU / Off CPU)
157+
self._track_state_transition(
158+
tid, on_cpu, self.on_cpu_start, self.off_cpu_start,
159+
"On CPU", "Off CPU", CATEGORY_CPU, current_time
160+
)
161+
162+
# Track code type (Python Code / Native Code)
163163
if has_gil:
164-
if tid not in self.python_code_start:
165-
self.python_code_start[tid] = current_time
166-
# End "Native Code" if it was running
167-
if tid in self.native_code_start:
168-
start_time = self.native_code_start[tid]
169-
self._add_marker(tid, "Native Code", start_time, current_time, CATEGORY_CODE_TYPE)
170-
del self.native_code_start[tid]
164+
self._track_state_transition(
165+
tid, True, self.python_code_start, self.native_code_start,
166+
"Python Code", "Native Code", CATEGORY_CODE_TYPE, current_time
167+
)
171168
elif on_cpu:
172-
# Native code: on CPU without GIL
173-
if tid not in self.native_code_start:
174-
self.native_code_start[tid] = current_time
175-
# End "Python Code" if it was running
176-
if tid in self.python_code_start:
177-
start_time = self.python_code_start[tid]
178-
self._add_marker(tid, "Python Code", start_time, current_time, CATEGORY_CODE_TYPE)
179-
del self.python_code_start[tid]
169+
self._track_state_transition(
170+
tid, True, self.native_code_start, self.python_code_start,
171+
"Native Code", "Python Code", CATEGORY_CODE_TYPE, current_time
172+
)
180173
else:
181174
# Neither has GIL nor on CPU - end both if running
182175
if tid in self.python_code_start:
183-
start_time = self.python_code_start[tid]
184-
self._add_marker(tid, "Python Code", start_time, current_time, CATEGORY_CODE_TYPE)
185-
del self.python_code_start[tid]
176+
self._add_marker(tid, "Python Code", self.python_code_start.pop(tid),
177+
current_time, CATEGORY_CODE_TYPE)
186178
if tid in self.native_code_start:
187-
start_time = self.native_code_start[tid]
188-
self._add_marker(tid, "Native Code", start_time, current_time, CATEGORY_CODE_TYPE)
189-
del self.native_code_start[tid]
179+
self._add_marker(tid, "Native Code", self.native_code_start.pop(tid),
180+
current_time, CATEGORY_CODE_TYPE)
190181

191-
# Track "Waiting for GIL" intervals
182+
# Track "Waiting for GIL" intervals (one-sided tracking)
192183
if gil_requested:
193-
if tid not in self.gil_wait_start:
194-
self.gil_wait_start[tid] = current_time
195-
else:
196-
if tid in self.gil_wait_start:
197-
start_time = self.gil_wait_start[tid]
198-
self._add_marker(tid, "Waiting for GIL", start_time, current_time, CATEGORY_GIL)
199-
del self.gil_wait_start[tid]
184+
self.gil_wait_start.setdefault(tid, current_time)
185+
elif tid in self.gil_wait_start:
186+
self._add_marker(tid, "Waiting for GIL", self.gil_wait_start.pop(tid),
187+
current_time, CATEGORY_GIL)
200188

201189
# Categorize: idle if neither has GIL nor on CPU
202190
is_idle = not has_gil and not on_cpu
@@ -546,41 +534,21 @@ def _finalize_markers(self):
546534
"""Close any open markers at the end of profiling."""
547535
end_time = self.last_sample_time
548536

549-
# Close all open markers for each thread
550-
for tid in list(self.has_gil_start.keys()):
551-
start_time = self.has_gil_start[tid]
552-
self._add_marker(tid, "Has GIL", start_time, end_time, CATEGORY_GIL)
553-
del self.has_gil_start[tid]
554-
555-
for tid in list(self.no_gil_start.keys()):
556-
start_time = self.no_gil_start[tid]
557-
self._add_marker(tid, "No GIL", start_time, end_time, CATEGORY_GIL)
558-
del self.no_gil_start[tid]
559-
560-
for tid in list(self.on_cpu_start.keys()):
561-
start_time = self.on_cpu_start[tid]
562-
self._add_marker(tid, "On CPU", start_time, end_time, CATEGORY_CPU)
563-
del self.on_cpu_start[tid]
564-
565-
for tid in list(self.off_cpu_start.keys()):
566-
start_time = self.off_cpu_start[tid]
567-
self._add_marker(tid, "Off CPU", start_time, end_time, CATEGORY_CPU)
568-
del self.off_cpu_start[tid]
569-
570-
for tid in list(self.python_code_start.keys()):
571-
start_time = self.python_code_start[tid]
572-
self._add_marker(tid, "Python Code", start_time, end_time, CATEGORY_CODE_TYPE)
573-
del self.python_code_start[tid]
574-
575-
for tid in list(self.native_code_start.keys()):
576-
start_time = self.native_code_start[tid]
577-
self._add_marker(tid, "Native Code", start_time, end_time, CATEGORY_CODE_TYPE)
578-
del self.native_code_start[tid]
579-
580-
for tid in list(self.gil_wait_start.keys()):
581-
start_time = self.gil_wait_start[tid]
582-
self._add_marker(tid, "Waiting for GIL", start_time, end_time, CATEGORY_GIL)
583-
del self.gil_wait_start[tid]
537+
# Close all open markers for each thread using a generic approach
538+
marker_states = [
539+
(self.has_gil_start, "Has GIL", CATEGORY_GIL),
540+
(self.no_gil_start, "No GIL", CATEGORY_GIL),
541+
(self.on_cpu_start, "On CPU", CATEGORY_CPU),
542+
(self.off_cpu_start, "Off CPU", CATEGORY_CPU),
543+
(self.python_code_start, "Python Code", CATEGORY_CODE_TYPE),
544+
(self.native_code_start, "Native Code", CATEGORY_CODE_TYPE),
545+
(self.gil_wait_start, "Waiting for GIL", CATEGORY_GIL),
546+
]
547+
548+
for state_dict, marker_name, category in marker_states:
549+
for tid in list(state_dict.keys()):
550+
self._add_marker(tid, marker_name, state_dict[tid], end_time, category)
551+
del state_dict[tid]
584552

585553
# Close any open GC marker
586554
if self.potential_gc_start is not None:

‎Modules/_remote_debugging_module.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,9 +2789,8 @@ unwind_stack_for_thread(
27892789
Py_DECREF(py_status);
27902790
goto error;
27912791
}
2792-
PyErr_Print();
27932792

2794-
// In PROFILING_MODE_ALL, py_status contains flags, otherwise it contains legacy enum
2793+
// py_status contains status flags (bitfield)
27952794
PyStructSequence_SetItem(result, 0, thread_id);
27962795
PyStructSequence_SetItem(result, 1, py_status); // Steals reference
27972796
PyStructSequence_SetItem(result, 2, frame_info); // Steals reference

0 commit comments

Comments
 (0)