Skip to content

Commit 42b25ad

Browse files
pablogsalambv
andauthored
gh-91048: Refactor and optimize remote debugging module (#134652)
Completely refactor Modules/_remote_debugging_module.c with improved code organization, replacing scattered reference counting and error handling with centralized goto error paths. This cleanup improves maintainability and reduces code duplication throughout the module while preserving the same external API. Implement memory page caching optimization in Python/remote_debug.h to avoid repeated reads of the same memory regions during debugging operations. The cache stores previously read memory pages and reuses them for subsequent reads, significantly reducing system calls and improving performance. Add code object caching mechanism with a new code_object_generation field in the interpreter state that tracks when code object caches need invalidation. This allows efficient reuse of parsed code object metadata and eliminates redundant processing of the same code objects across debugging sessions. Optimize memory operations by replacing multiple individual structure copies with single bulk reads for the same data structures. This reduces the number of memory operations and system calls required to gather debugging information from the target process. Update Makefile.pre.in to include Python/remote_debug.h in the headers list, ensuring that changes to the remote debugging header force proper recompilation of dependent modules and maintain build consistency across the codebase. Also, make the module compatible with the free threading build as an extra :) Co-authored-by: Łukasz Langa <[email protected]>
1 parent 328a778 commit 42b25ad

16 files changed

+2390
-1058
lines changed

Include/cpython/pystate.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ typedef struct _stack_chunk {
6161
PyObject * data[1]; /* Variable sized */
6262
} _PyStackChunk;
6363

64+
/* Minimum size of data stack chunk */
65+
#define _PY_DATA_STACK_CHUNK_SIZE (16*1024)
6466
struct _ts {
6567
/* See Python/ceval.c for comments explaining most fields */
6668

Include/internal/pycore_debug_offsets.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ extern "C" {
5454
# define _Py_Debug_Free_Threaded 1
5555
# define _Py_Debug_code_object_co_tlbc offsetof(PyCodeObject, co_tlbc)
5656
# define _Py_Debug_interpreter_frame_tlbc_index offsetof(_PyInterpreterFrame, tlbc_index)
57+
# define _Py_Debug_interpreter_state_tlbc_generation offsetof(PyInterpreterState, tlbc_indices.tlbc_generation)
5758
#else
5859
# define _Py_Debug_gilruntimestate_enabled 0
5960
# define _Py_Debug_Free_Threaded 0
6061
# define _Py_Debug_code_object_co_tlbc 0
6162
# define _Py_Debug_interpreter_frame_tlbc_index 0
63+
# define _Py_Debug_interpreter_state_tlbc_generation 0
6264
#endif
6365

6466

@@ -89,6 +91,8 @@ typedef struct _Py_DebugOffsets {
8991
uint64_t gil_runtime_state_enabled;
9092
uint64_t gil_runtime_state_locked;
9193
uint64_t gil_runtime_state_holder;
94+
uint64_t code_object_generation;
95+
uint64_t tlbc_generation;
9296
} interpreter_state;
9397

9498
// Thread state offset;
@@ -216,6 +220,11 @@ typedef struct _Py_DebugOffsets {
216220
uint64_t gi_frame_state;
217221
} gen_object;
218222

223+
struct _llist_node {
224+
uint64_t next;
225+
uint64_t prev;
226+
} llist_node;
227+
219228
struct _debugger_support {
220229
uint64_t eval_breaker;
221230
uint64_t remote_debugger_support;
@@ -251,6 +260,8 @@ typedef struct _Py_DebugOffsets {
251260
.gil_runtime_state_enabled = _Py_Debug_gilruntimestate_enabled, \
252261
.gil_runtime_state_locked = offsetof(PyInterpreterState, _gil.locked), \
253262
.gil_runtime_state_holder = offsetof(PyInterpreterState, _gil.last_holder), \
263+
.code_object_generation = offsetof(PyInterpreterState, _code_object_generation), \
264+
.tlbc_generation = _Py_Debug_interpreter_state_tlbc_generation, \
254265
}, \
255266
.thread_state = { \
256267
.size = sizeof(PyThreadState), \
@@ -347,6 +358,10 @@ typedef struct _Py_DebugOffsets {
347358
.gi_iframe = offsetof(PyGenObject, gi_iframe), \
348359
.gi_frame_state = offsetof(PyGenObject, gi_frame_state), \
349360
}, \
361+
.llist_node = { \
362+
.next = offsetof(struct llist_node, next), \
363+
.prev = offsetof(struct llist_node, prev), \
364+
}, \
350365
.debugger_support = { \
351366
.eval_breaker = offsetof(PyThreadState, eval_breaker), \
352367
.remote_debugger_support = offsetof(PyThreadState, remote_debugger_support), \

Include/internal/pycore_global_objects_fini_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_global_strings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ struct _Py_global_strings {
286286
STRUCT_FOR_ID(alias)
287287
STRUCT_FOR_ID(align)
288288
STRUCT_FOR_ID(all)
289+
STRUCT_FOR_ID(all_threads)
289290
STRUCT_FOR_ID(allow_code)
290291
STRUCT_FOR_ID(any)
291292
STRUCT_FOR_ID(append)

Include/internal/pycore_interp_structs.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,10 @@ typedef struct _PyIndexPool {
726726

727727
// Next index to allocate if no free indices are available
728728
int32_t next_index;
729+
730+
// Generation counter incremented on thread creation/destruction
731+
// Used for TLBC cache invalidation in remote debugging
732+
uint32_t tlbc_generation;
729733
} _PyIndexPool;
730734

731735
typedef union _Py_unique_id_entry {
@@ -843,6 +847,8 @@ struct _is {
843847
/* The per-interpreter GIL, which might not be used. */
844848
struct _gil_runtime_state _gil;
845849

850+
uint64_t _code_object_generation;
851+
846852
/* ---------- IMPORTANT ---------------------------
847853
The fields above this line are declared as early as
848854
possible to facilitate out-of-process observability

Include/internal/pycore_runtime_init_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_unicodeobject_generated.h

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/asyncio/tools.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
"""Tools to analyze tasks running in asyncio programs."""
22

3-
from dataclasses import dataclass
43
from collections import defaultdict
54
from itertools import count
65
from enum import Enum
76
import sys
8-
from _remote_debugging import get_all_awaited_by
7+
from _remote_debugging import RemoteUnwinder
98

109

1110
class NodeType(Enum):
@@ -118,6 +117,11 @@ def dfs(v):
118117

119118

120119
# ─── PRINT TREE FUNCTION ───────────────────────────────────────
120+
def get_all_awaited_by(pid):
121+
unwinder = RemoteUnwinder(pid)
122+
return unwinder.get_all_awaited_by()
123+
124+
121125
def build_async_tree(result, task_emoji="(T)", cor_emoji=""):
122126
"""
123127
Build a list of strings for pretty-print an async call tree.

Lib/test/test_external_inspection.py

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import importlib
55
import sys
66
import socket
7+
import threading
78
from asyncio import staggered, taskgroups
89
from unittest.mock import ANY
910
from test.support import os_helper, SHORT_TIMEOUT, busy_retry
@@ -16,9 +17,7 @@
1617

1718
try:
1819
from _remote_debugging import PROCESS_VM_READV_SUPPORTED
19-
from _remote_debugging import get_stack_trace
20-
from _remote_debugging import get_async_stack_trace
21-
from _remote_debugging import get_all_awaited_by
20+
from _remote_debugging import RemoteUnwinder
2221
except ImportError:
2322
raise unittest.SkipTest("Test only runs when _remote_debugging is available")
2423

@@ -34,7 +33,23 @@ def _make_test_script(script_dir, script_basename, source):
3433
)
3534

3635

36+
def get_stack_trace(pid):
37+
unwinder = RemoteUnwinder(pid, all_threads=True)
38+
return unwinder.get_stack_trace()
39+
40+
41+
def get_async_stack_trace(pid):
42+
unwinder = RemoteUnwinder(pid)
43+
return unwinder.get_async_stack_trace()
44+
45+
46+
def get_all_awaited_by(pid):
47+
unwinder = RemoteUnwinder(pid)
48+
return unwinder.get_all_awaited_by()
49+
50+
3751
class TestGetStackTrace(unittest.TestCase):
52+
maxDiff = None
3853

3954
@skip_if_not_supported
4055
@unittest.skipIf(
@@ -46,7 +61,7 @@ def test_remote_stack_trace(self):
4661
port = find_unused_port()
4762
script = textwrap.dedent(
4863
f"""\
49-
import time, sys, socket
64+
import time, sys, socket, threading
5065
# Connect to the test process
5166
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
5267
sock.connect(('localhost', {port}))
@@ -55,13 +70,16 @@ def bar():
5570
for x in range(100):
5671
if x == 50:
5772
baz()
73+
5874
def baz():
5975
foo()
6076
6177
def foo():
62-
sock.sendall(b"ready"); time.sleep(10_000) # same line number
78+
sock.sendall(b"ready:thread\\n"); time.sleep(10_000) # same line number
6379
64-
bar()
80+
t = threading.Thread(target=bar)
81+
t.start()
82+
sock.sendall(b"ready:main\\n"); t.join() # same line number
6583
"""
6684
)
6785
stack_trace = None
@@ -82,8 +100,9 @@ def foo():
82100
p = subprocess.Popen([sys.executable, script_name])
83101
client_socket, _ = server_socket.accept()
84102
server_socket.close()
85-
response = client_socket.recv(1024)
86-
self.assertEqual(response, b"ready")
103+
response = b""
104+
while b"ready:main" not in response or b"ready:thread" not in response:
105+
response += client_socket.recv(1024)
87106
stack_trace = get_stack_trace(p.pid)
88107
except PermissionError:
89108
self.skipTest("Insufficient permissions to read the stack trace")
@@ -94,13 +113,23 @@ def foo():
94113
p.terminate()
95114
p.wait(timeout=SHORT_TIMEOUT)
96115

97-
expected_stack_trace = [
98-
("foo", script_name, 14),
99-
("baz", script_name, 11),
116+
thread_expected_stack_trace = [
117+
("foo", script_name, 15),
118+
("baz", script_name, 12),
100119
("bar", script_name, 9),
101-
("<module>", script_name, 16),
120+
('Thread.run', threading.__file__, ANY)
102121
]
103-
self.assertEqual(stack_trace, expected_stack_trace)
122+
# Is possible that there are more threads, so we check that the
123+
# expected stack traces are in the result (looking at you Windows!)
124+
self.assertIn((ANY, thread_expected_stack_trace), stack_trace)
125+
126+
# Check that the main thread stack trace is in the result
127+
frame = ("<module>", script_name, 19)
128+
for _, stack in stack_trace:
129+
if frame in stack:
130+
break
131+
else:
132+
self.fail("Main thread stack trace not found in result")
104133

105134
@skip_if_not_supported
106135
@unittest.skipIf(
@@ -700,13 +729,28 @@ async def main():
700729
)
701730
def test_self_trace(self):
702731
stack_trace = get_stack_trace(os.getpid())
732+
# Is possible that there are more threads, so we check that the
733+
# expected stack traces are in the result (looking at you Windows!)
734+
this_tread_stack = None
735+
for thread_id, stack in stack_trace:
736+
if thread_id == threading.get_native_id():
737+
this_tread_stack = stack
738+
break
739+
self.assertIsNotNone(this_tread_stack)
703740
self.assertEqual(
704-
stack_trace[0],
705-
(
706-
"TestGetStackTrace.test_self_trace",
707-
__file__,
708-
self.test_self_trace.__code__.co_firstlineno + 6,
709-
),
741+
stack[:2],
742+
[
743+
(
744+
"get_stack_trace",
745+
__file__,
746+
get_stack_trace.__code__.co_firstlineno + 2,
747+
),
748+
(
749+
"TestGetStackTrace.test_self_trace",
750+
__file__,
751+
self.test_self_trace.__code__.co_firstlineno + 6,
752+
),
753+
]
710754
)
711755

712756

Makefile.pre.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,7 @@ PYTHON_HEADERS= \
12061206
$(srcdir)/Include/unicodeobject.h \
12071207
$(srcdir)/Include/warnings.h \
12081208
$(srcdir)/Include/weakrefobject.h \
1209+
$(srcdir)/Python/remote_debug.h \
12091210
\
12101211
pyconfig.h \
12111212
$(PARSER_HEADERS) \

0 commit comments

Comments
 (0)