Skip to content

Commit ad5b78c

Browse files
committed
profiling(gecko): use status flag for main thread instead of InterpreterInfo field
Replace the main_thread_id field in InterpreterInfo with a THREAD_STATUS_MAIN_THREAD status flag set directly in the thread unwinding code. This simplifies the struct from 3 to 2 fields and removes the need to compare thread IDs at the Python level.
1 parent 5849aad commit ad5b78c

10 files changed

Lines changed: 62 additions & 31 deletions

File tree

Lib/profiling/sampling/constants.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
THREAD_STATUS_UNKNOWN,
3838
THREAD_STATUS_GIL_REQUESTED,
3939
THREAD_STATUS_HAS_EXCEPTION,
40+
THREAD_STATUS_MAIN_THREAD,
4041
)
4142
except ImportError:
4243
# Fallback for tests or when module is not available
@@ -45,3 +46,4 @@
4546
THREAD_STATUS_UNKNOWN = (1 << 2)
4647
THREAD_STATUS_GIL_REQUESTED = (1 << 3)
4748
THREAD_STATUS_HAS_EXCEPTION = (1 << 4)
49+
THREAD_STATUS_MAIN_THREAD = (1 << 5)

Lib/profiling/sampling/gecko_collector.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
from .collector import Collector, filter_internal_frames
1010
from .opcode_utils import get_opcode_info, format_opcode
1111
try:
12-
from _remote_debugging import THREAD_STATUS_HAS_GIL, THREAD_STATUS_ON_CPU, THREAD_STATUS_UNKNOWN, THREAD_STATUS_GIL_REQUESTED, THREAD_STATUS_HAS_EXCEPTION
12+
from _remote_debugging import THREAD_STATUS_HAS_GIL, THREAD_STATUS_ON_CPU, THREAD_STATUS_UNKNOWN, THREAD_STATUS_GIL_REQUESTED, THREAD_STATUS_HAS_EXCEPTION, THREAD_STATUS_MAIN_THREAD
1313
except ImportError:
1414
# Fallback if module not available (shouldn't happen in normal use)
1515
THREAD_STATUS_HAS_GIL = (1 << 0)
1616
THREAD_STATUS_ON_CPU = (1 << 1)
1717
THREAD_STATUS_UNKNOWN = (1 << 2)
1818
THREAD_STATUS_GIL_REQUESTED = (1 << 3)
1919
THREAD_STATUS_HAS_EXCEPTION = (1 << 4)
20+
THREAD_STATUS_MAIN_THREAD = (1 << 5)
2021

2122

2223
# Categories matching Firefox Profiler expectations
@@ -171,19 +172,19 @@ def collect(self, stack_frames, timestamps_us=None):
171172

172173
# Process threads
173174
for interpreter_info in stack_frames:
174-
main_tid = interpreter_info.main_thread_id
175175
for thread_info in interpreter_info.threads:
176176
frames = filter_internal_frames(thread_info.frame_info)
177177
tid = thread_info.thread_id
178+
status_flags = thread_info.status
179+
is_main_thread = bool(status_flags & THREAD_STATUS_MAIN_THREAD)
178180

179181
# Initialize thread if needed
180182
if tid not in self.threads:
181-
self.threads[tid] = self._create_thread(tid, main_tid)
183+
self.threads[tid] = self._create_thread(tid, is_main_thread)
182184

183185
thread_data = self.threads[tid]
184186

185187
# Decode status flags
186-
status_flags = thread_info.status
187188
has_gil = bool(status_flags & THREAD_STATUS_HAS_GIL)
188189
on_cpu = bool(status_flags & THREAD_STATUS_ON_CPU)
189190
gil_requested = bool(status_flags & THREAD_STATUS_GIL_REQUESTED)
@@ -289,14 +290,12 @@ def collect(self, stack_frames, timestamps_us=None):
289290

290291
self.sample_count += len(times)
291292

292-
def _create_thread(self, tid, main_tid):
293+
def _create_thread(self, tid, is_main_thread):
293294
"""Create a new thread structure with processed profile format."""
294295

295-
is_main = tid == main_tid
296-
297296
thread = {
298297
"name": f"Thread-{tid}",
299-
"isMainThread": is_main,
298+
"isMainThread": is_main_thread,
300299
"processStartupTime": 0,
301300
"processShutdownTime": None,
302301
"registerTime": 0,

Lib/test/test_profiling/test_sampling_profiler/mocks.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,12 @@ def __repr__(self):
5050
class MockInterpreterInfo:
5151
"""Mock InterpreterInfo for testing since the real one isn't accessible."""
5252

53-
def __init__(self, interpreter_id, threads, main_thread_id=None):
53+
def __init__(self, interpreter_id, threads):
5454
self.interpreter_id = interpreter_id
55-
self.main_thread_id = main_thread_id
5655
self.threads = threads
5756

5857
def __repr__(self):
59-
return f"MockInterpreterInfo(interpreter_id={self.interpreter_id}, main_thread_id={self.main_thread_id}, threads={self.threads})"
58+
return f"MockInterpreterInfo(interpreter_id={self.interpreter_id}, threads={self.threads})"
6059

6160

6261
class MockCoroInfo:

Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
THREAD_STATUS_UNKNOWN,
1919
THREAD_STATUS_GIL_REQUESTED,
2020
THREAD_STATUS_HAS_EXCEPTION,
21+
THREAD_STATUS_MAIN_THREAD,
2122
)
2223
from profiling.sampling.binary_collector import BinaryCollector
2324
from profiling.sampling.binary_reader import BinaryReader
25+
from profiling.sampling.gecko_collector import GeckoCollector
2426

2527
ZSTD_AVAILABLE = _remote_debugging.zstd_available()
2628
except ImportError:
@@ -47,9 +49,9 @@ def make_thread(thread_id, frames, status=0):
4749
return ThreadInfo((thread_id, status, frames))
4850

4951

50-
def make_interpreter(interp_id, threads, main_thread_id=0):
52+
def make_interpreter(interp_id, threads):
5153
"""Create an InterpreterInfo struct sequence."""
52-
return InterpreterInfo((interp_id, main_thread_id, threads))
54+
return InterpreterInfo((interp_id, threads))
5355

5456

5557
def extract_lineno(location):
@@ -318,6 +320,7 @@ def test_status_flags_preserved(self):
318320
THREAD_STATUS_UNKNOWN,
319321
THREAD_STATUS_GIL_REQUESTED,
320322
THREAD_STATUS_HAS_EXCEPTION,
323+
THREAD_STATUS_MAIN_THREAD,
321324
THREAD_STATUS_HAS_GIL | THREAD_STATUS_ON_CPU,
322325
THREAD_STATUS_HAS_GIL | THREAD_STATUS_HAS_EXCEPTION,
323326
THREAD_STATUS_HAS_GIL
@@ -342,6 +345,35 @@ def test_status_flags_preserved(self):
342345
self.assertEqual(count, len(statuses))
343346
self.assert_samples_equal(samples, collector)
344347

348+
def test_binary_replay_preserves_main_thread_for_gecko(self):
349+
"""Binary replay preserves main thread identity for GeckoCollector."""
350+
samples = [
351+
[
352+
make_interpreter(
353+
0,
354+
[
355+
make_thread(
356+
1,
357+
[make_frame("main.py", 10, "main")],
358+
THREAD_STATUS_MAIN_THREAD,
359+
),
360+
make_thread(2, [make_frame("worker.py", 20, "worker")]),
361+
],
362+
)
363+
]
364+
]
365+
filename = self.create_binary_file(samples)
366+
collector = GeckoCollector(1000)
367+
368+
with BinaryReader(filename) as reader:
369+
count = reader.replay_samples(collector)
370+
371+
self.assertEqual(count, 2)
372+
profile = collector._build_profile()
373+
threads = {thread["tid"]: thread for thread in profile["threads"]}
374+
self.assertTrue(threads[1]["isMainThread"])
375+
self.assertFalse(threads[2]["isMainThread"])
376+
345377
def test_multiple_threads_per_sample(self):
346378
"""Multiple threads in one sample roundtrip exactly."""
347379
threads = [

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
THREAD_STATUS_HAS_GIL,
2929
THREAD_STATUS_ON_CPU,
3030
THREAD_STATUS_GIL_REQUESTED,
31+
THREAD_STATUS_MAIN_THREAD,
3132
)
3233
except ImportError:
3334
raise unittest.SkipTest(
@@ -524,9 +525,9 @@ def test_gecko_collector_basic(self):
524525
MockThreadInfo(
525526
1,
526527
[MockFrameInfo("file.py", 10, "func1"), MockFrameInfo("file.py", 20, "func2")],
528+
status=THREAD_STATUS_MAIN_THREAD,
527529
)
528530
],
529-
main_thread_id=1,
530531
)
531532
]
532533
collector.collect(test_frames)

Modules/_remote_debugging/_remote_debugging.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ typedef enum _WIN32_THREADSTATE {
172172
#define THREAD_STATUS_UNKNOWN (1 << 2)
173173
#define THREAD_STATUS_GIL_REQUESTED (1 << 3)
174174
#define THREAD_STATUS_HAS_EXCEPTION (1 << 4)
175+
#define THREAD_STATUS_MAIN_THREAD (1 << 5)
175176

176177
/* Exception cause macro */
177178
#define set_exception_cause(unwinder, exc_type, message) \
@@ -576,7 +577,7 @@ extern PyObject* unwind_stack_for_thread(
576577
uintptr_t *current_tstate,
577578
uintptr_t gil_holder_tstate,
578579
uintptr_t gc_frame,
579-
uint64_t *current_thread_id
580+
uintptr_t main_thread_tstate
580581
);
581582

582583
/* Thread stopping functions (for blocking mode) */

Modules/_remote_debugging/binary_io_reader.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ build_sample_list(RemoteDebuggingState *state, BinaryReader *reader,
828828
goto error;
829829
}
830830
PyStructSequence_SetItem(interp_info, 0, iid);
831-
PyStructSequence_SetItem(interp_info, 2, thread_list);
831+
PyStructSequence_SetItem(interp_info, 1, thread_list);
832832
thread_list = NULL;
833833

834834
sample_list = PyList_New(1);

Modules/_remote_debugging/binary_io_writer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ binary_writer_write_sample(BinaryWriter *writer, PyObject *stack_frames, uint64_
10081008
PyObject *interp_info = PyList_GET_ITEM(stack_frames, i);
10091009

10101010
PyObject *interp_id_obj = PyStructSequence_GET_ITEM(interp_info, 0);
1011-
PyObject *threads = PyStructSequence_GET_ITEM(interp_info, 2);
1011+
PyObject *threads = PyStructSequence_GET_ITEM(interp_info, 1);
10121012

10131013
unsigned long interp_id_long = PyLong_AsUnsignedLong(interp_id_obj);
10141014
if (interp_id_long == (unsigned long)-1 && PyErr_Occurred()) {

Modules/_remote_debugging/module.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ PyStructSequence_Desc ThreadInfo_desc = {
106106
// InterpreterInfo structseq type
107107
static PyStructSequence_Field InterpreterInfo_fields[] = {
108108
{"interpreter_id", "Interpreter ID"},
109-
{"main_thread_id", "Main thread ID"},
110109
{"threads", "List of threads in this interpreter"},
111110
{NULL}
112111
};
@@ -115,7 +114,7 @@ PyStructSequence_Desc InterpreterInfo_desc = {
115114
"_remote_debugging.InterpreterInfo",
116115
"Information about an interpreter",
117116
InterpreterInfo_fields,
118-
3
117+
2
119118
};
120119

121120
// AwaitedInfo structseq type
@@ -588,15 +587,12 @@ _remote_debugging_RemoteUnwinder_get_stack_trace_impl(RemoteUnwinderObject *self
588587
uintptr_t main_thread_tstate = GET_MEMBER(uintptr_t, interp_state_buffer,
589588
self->debug_offsets.interpreter_state.threads_main);
590589

591-
PyObject *main_thread_id = NULL;
592-
593-
uint64_t prev_thread_id = 0;
594590
while (current_tstate != 0) {
595591
uintptr_t prev_tstate = current_tstate;
596592
PyObject* frame_info = unwind_stack_for_thread(self, &current_tstate,
597593
gil_holder_tstate,
598594
gc_frame,
599-
&prev_thread_id);
595+
main_thread_tstate);
600596
if (!frame_info) {
601597
// Check if this was an intentional skip due to mode-based filtering
602598
if ((self->mode == PROFILING_MODE_CPU || self->mode == PROFILING_MODE_GIL ||
@@ -622,10 +618,6 @@ _remote_debugging_RemoteUnwinder_get_stack_trace_impl(RemoteUnwinderObject *self
622618
goto exit;
623619
}
624620

625-
if (prev_tstate == main_thread_tstate) {
626-
main_thread_id = PyLong_FromUnsignedLongLong(prev_thread_id);
627-
}
628-
629621
if (PyList_Append(interpreter_threads, frame_info) == -1) {
630622
Py_DECREF(frame_info);
631623
Py_DECREF(interpreter_threads);
@@ -661,8 +653,7 @@ _remote_debugging_RemoteUnwinder_get_stack_trace_impl(RemoteUnwinderObject *self
661653
}
662654

663655
PyStructSequence_SetItem(interpreter_info, 0, interp_id); // steals reference
664-
PyStructSequence_SetItem(interpreter_info, 1, main_thread_id); // steals reference
665-
PyStructSequence_SetItem(interpreter_info, 2, interpreter_threads); // steals reference
656+
PyStructSequence_SetItem(interpreter_info, 1, interpreter_threads); // steals reference
666657

667658
// Add this interpreter to the result list
668659
if (PyList_Append(result, interpreter_info) == -1) {
@@ -1221,6 +1212,9 @@ _remote_debugging_exec(PyObject *m)
12211212
if (PyModule_AddIntConstant(m, "THREAD_STATUS_HAS_EXCEPTION", THREAD_STATUS_HAS_EXCEPTION) < 0) {
12221213
return -1;
12231214
}
1215+
if (PyModule_AddIntConstant(m, "THREAD_STATUS_MAIN_THREAD", THREAD_STATUS_MAIN_THREAD) < 0) {
1216+
return -1;
1217+
}
12241218

12251219
if (RemoteDebugging_InitState(st) < 0) {
12261220
return -1;

Modules/_remote_debugging/threads.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ unwind_stack_for_thread(
292292
uintptr_t *current_tstate,
293293
uintptr_t gil_holder_tstate,
294294
uintptr_t gc_frame,
295-
uint64_t *current_tid
295+
uintptr_t main_thread_tstate
296296
) {
297297
PyObject *frame_info = NULL;
298298
PyObject *thread_id = NULL;
@@ -310,7 +310,6 @@ unwind_stack_for_thread(
310310
STATS_ADD(unwinder, memory_bytes_read, unwinder->debug_offsets.thread_state.size);
311311

312312
long tid = GET_MEMBER(long, ts, unwinder->debug_offsets.thread_state.native_thread_id);
313-
*current_tid = tid;
314313

315314
// Read GC collecting state from the interpreter (before any skip checks)
316315
uintptr_t interp_addr = GET_MEMBER(uintptr_t, ts, unwinder->debug_offsets.thread_state.interp);
@@ -397,6 +396,10 @@ unwind_stack_for_thread(
397396
status_flags |= THREAD_STATUS_ON_CPU;
398397
}
399398

399+
if (*current_tstate == main_thread_tstate) {
400+
status_flags |= THREAD_STATUS_MAIN_THREAD;
401+
}
402+
400403
// Check if we should skip this thread based on mode
401404
int should_skip = 0;
402405
if (unwinder->skip_non_matching_threads) {

0 commit comments

Comments
 (0)