cleanp

Author: pablogsal

Modules/_remote_debugging/_remote_debugging.h

@@ -30,6 +30,7 @@ extern "C" {
 #include "internal/pycore_llist.h"          // struct llist_node
 #include "internal/pycore_long.h"           // _PyLong_GetZero
 #include "internal/pycore_pyerrors.h"       // _PyErr_FormatFromCause
+#include "internal/pycore_pyhash.h"        // _Py_HashPointerRaw
 #include "internal/pycore_stackref.h"       // Py_TAG_BITS
 #include "../../Python/remote_debug.h"
 
@@ -270,16 +271,22 @@ typedef struct {
     uint64_t batched_read_segments_completed; // Segments completed by batched reads
 } UnwinderStats;
 
+#if defined(__GNUC__) || defined(__clang__)
+#  define REMOTE_DEBUG_UNLIKELY(value) __builtin_expect(!!(value), 0)
+#else
+#  define REMOTE_DEBUG_UNLIKELY(value) (value)
+#endif
+
 /* Stats tracking macros - no-op when stats collection is disabled */
 #define STATS_INC(unwinder, field) \
-    do { if ((unwinder)->collect_stats) (unwinder)->stats.field++; } while(0)
+    do { if (REMOTE_DEBUG_UNLIKELY((unwinder)->collect_stats)) (unwinder)->stats.field++; } while(0)
 
 #define STATS_ADD(unwinder, field, val) \
-    do { if ((unwinder)->collect_stats) (unwinder)->stats.field += (val); } while(0)
+    do { if (REMOTE_DEBUG_UNLIKELY((unwinder)->collect_stats)) (unwinder)->stats.field += (val); } while(0)
 
 #define STATS_BATCHED_READ(unwinder, requested, completed) \
     do { \
-        if ((unwinder)->collect_stats) { \
+        if (REMOTE_DEBUG_UNLIKELY((unwinder)->collect_stats)) { \
             (unwinder)->stats.batched_read_attempts++; \
             (unwinder)->stats.batched_read_segments_requested += (uint64_t)(requested); \
             (unwinder)->stats.batched_read_segments_completed += (uint64_t)(completed); \
@@ -292,31 +299,6 @@ typedef struct {
         } \
     } while(0)
 
-static inline int
-_Py_RemoteDebug_CountCompletedSegments(
-    const _Py_RemoteReadSegment *segments,
-    int nsegs,
-    Py_ssize_t nread)
-{
-    if (nread < 0) {
-        return 0;
-    }
-
-    int completed = 0;
-    Py_ssize_t bytes_needed = 0;
-    for (int i = 0; i < nsegs; i++) {
-        if (segments[i].size > (size_t)(PY_SSIZE_T_MAX - bytes_needed)) {
-            break;
-        }
-        bytes_needed += (Py_ssize_t)segments[i].size;
-        if (nread < bytes_needed) {
-            break;
-        }
-        completed++;
-    }
-    return completed;
-}
-
 typedef struct {
     PyTypeObject *RemoteDebugging_Type;
     PyTypeObject *TaskInfo_Type;
@@ -368,10 +350,12 @@ typedef struct {
     int cache_frames;
     int collect_stats;  // whether to collect statistics
     uint32_t stale_invalidation_counter;  // counter for throttling frame_cache_invalidate_stale
-    InterpreterThreadCacheEntry cached_tstates[INTERPRETER_THREAD_CACHE_SIZE];
+    uintptr_t cached_tstate_interpreter_addr;  // hot last-interpreter prediction
+    uintptr_t cached_tstate_addr;  // hot first-thread prediction
     RemoteDebuggingState *cached_state;
     FrameCacheEntry *frame_cache;  // preallocated array of FRAME_CACHE_MAX_THREADS entries
     UnwinderStats stats;  // statistics for performance analysis
+    InterpreterThreadCacheEntry cached_tstates[INTERPRETER_THREAD_CACHE_SIZE];
 #ifdef Py_GIL_DISABLED
     uint32_t tlbc_generation;
     _Py_hashtable_t *tlbc_cache;

Modules/_remote_debugging/module.c

@@ -361,6 +361,8 @@ _remote_debugging_RemoteUnwinder___init___impl(RemoteUnwinderObject *self,
     self->cache_frames = cache_frames;
     self->collect_stats = stats;
     self->stale_invalidation_counter = 0;
+    self->cached_tstate_interpreter_addr = 0;
+    self->cached_tstate_addr = 0;
     memset(self->cached_tstates, 0, sizeof(self->cached_tstates));
     self->debug = debug;
     self->only_active_thread = only_active_thread;
@@ -478,13 +480,10 @@ _remote_debugging_RemoteUnwinder___init___impl(RemoteUnwinderObject *self,
 static size_t
 interpreter_thread_cache_index(uintptr_t interpreter_addr)
 {
-    // The interpreter ID lives in PyInterpreterState, which is the state we are
-    // trying to prefetch. At this point the cheap stable key is the remote
-    // interpreter address, so use it for a small direct-mapped prediction cache.
-    // The full address is stored in each entry and checked on lookup, so hash
-    // collisions are detected as misses; storing a colliding entry only replaces
-    // the previous prediction and cannot return the wrong thread state.
-    return ((interpreter_addr >> 4) ^ (interpreter_addr >> 12))
+    // Direct-mapped table indexed by the remote interpreter address. Each entry
+    // stores the full address and verifies it on lookup, so hash collisions
+    // degrade to misses and cannot return a wrong tstate.
+    return (size_t)_Py_HashPointerRaw((const void *)interpreter_addr)
         & (INTERPRETER_THREAD_CACHE_SIZE - 1);
 }
 
@@ -497,9 +496,15 @@ get_cached_tstate_for_interpreter(
         return 0;
     }
 
+    if (self->cached_tstate_interpreter_addr == interpreter_addr) {
+        return self->cached_tstate_addr;
+    }
+
     InterpreterThreadCacheEntry *entry =
         &self->cached_tstates[interpreter_thread_cache_index(interpreter_addr)];
     if (entry->interpreter_addr == interpreter_addr) {
+        self->cached_tstate_interpreter_addr = interpreter_addr;
+        self->cached_tstate_addr = entry->thread_state_addr;
         return entry->thread_state_addr;
     }
     return 0;
@@ -515,6 +520,9 @@ set_cached_tstate_for_interpreter(
         return;
     }
 
+    self->cached_tstate_interpreter_addr = interpreter_addr;
+    self->cached_tstate_addr = thread_state_addr;
+
     InterpreterThreadCacheEntry *entry =
         &self->cached_tstates[interpreter_thread_cache_index(interpreter_addr)];
     entry->interpreter_addr = interpreter_addr;
@@ -584,8 +592,19 @@ read_interp_state_and_maybe_thread_frame(
         int nsegs = prefetch->frame_addr != 0 ? 3 : 2;
         Py_ssize_t nread = _Py_RemoteDebug_BatchedReadRemoteMemory(
             &unwinder->handle, segments, nsegs);
-        int completed = _Py_RemoteDebug_CountCompletedSegments(
-            segments, nsegs, nread);
+        int completed = 0;
+        if (nread >= (Py_ssize_t)INTERP_STATE_BUFFER_SIZE) {
+            completed = 1;
+            Py_ssize_t with_tstate = (Py_ssize_t)INTERP_STATE_BUFFER_SIZE
+                + (Py_ssize_t)tstate_size;
+            if (nread >= with_tstate) {
+                completed = 2;
+            }
+            if (nsegs == 3
+                    && nread == with_tstate + (Py_ssize_t)SIZEOF_INTERP_FRAME) {
+                completed = 3;
+            }
+        }
         STATS_BATCHED_READ(unwinder, nsegs, completed);
         if (completed >= 1) {
             if (completed >= 2) {

Modules/_remote_debugging/threads.c

@@ -307,7 +307,13 @@ read_thread_state_and_maybe_frame(
         };
         Py_ssize_t nread = _Py_RemoteDebug_BatchedReadRemoteMemory(
             &unwinder->handle, segments, 2);
-        int completed = _Py_RemoteDebug_CountCompletedSegments(segments, 2, nread);
+        int completed = 0;
+        if (nread >= (Py_ssize_t)tstate_size) {
+            completed = 1;
+            if (nread == (Py_ssize_t)(tstate_size + SIZEOF_INTERP_FRAME)) {
+                completed = 2;
+            }
+        }
         STATS_BATCHED_READ(unwinder, 2, completed);
         if (completed >= 1) {
             *frame_read = completed == 2;
@@ -335,9 +341,9 @@ unwind_stack_for_thread(
     char ts[SIZEOF_THREAD_STATE];
     char local_prefetched_frame[SIZEOF_INTERP_FRAME];
     RemoteReadPrefetch ctx_prefetch = {0};
-    if (prefetch && prefetch->tstate && prefetch->tstate_addr == *current_tstate) {
+    if (prefetch->tstate && prefetch->tstate_addr == *current_tstate) {
         memcpy(ts, prefetch->tstate, (size_t)unwinder->debug_offsets.thread_state.size);
-        if (prefetch->frame && prefetch->frame_addr != 0) {
+        if (prefetch->frame) {
             ctx_prefetch.frame = prefetch->frame;
             ctx_prefetch.frame_addr = prefetch->frame_addr;
         }