gc, vm: address review notes on owner tags and interpreter docs

Look up retired owner tags with a sorted binary search instead of a linear scan, which every scanned object paid for once per dropped interpreter. Drop the claim that clearing a tag frees it for reuse; `alloc_owner` only ever hands out new tags. Also correct the tag space it mentions, which is 16-bit since the tag was sized to the header padding. Document `is_main` as "top-level interpreter" rather than "the process main": every top-level interpreter sets it, and only the first registered one becomes the main `main_interpreter_id` reports. Assert membership rather than an exact owned-interpreter count delta; the owned table is process-global and other tests store into it in parallel. Assisted-by: Claude
youknowone · Aug 16, 2026 · Aug 13, 2026 · Aug 13, 2026 · Aug 13, 2026 · Aug 13, 2026
commit c3fcc702e09d474abfdc10b56711114a41066a7d
diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs
@@ -306,7 +306,7 @@ impl GcState {
     }
 
     /// Reserve a tag for a new interpreter. Tags are never reused; exhausting
-    /// the 32-bit space falls back to `GC_NO_OWNER`, which costs isolation but
+    /// the tag space falls back to `GC_NO_OWNER`, which costs isolation but
     /// stays correct, rather than aliasing a live interpreter.
     fn alloc_owner(&self) -> GcOwner {
         self.next_owner
@@ -536,11 +536,18 @@ impl GcState {
         // Another interpreter's objects stay out of the candidate set, so they
         // act as external roots: anything they reference survives this pass.
         let owner = gc.owner;
-        let retired = self.retired.lock().clone();
+        // Sorted so that the test below, which every scanned object pays for,
+        // stays logarithmic in the number of interpreters that have been
+        // dropped instead of linear.
+        let retired = {
+            let mut retired = self.retired.lock().clone();
+            retired.sort_unstable();
+            retired
+        };
         let mut collecting: HashSet<GcPtr> = HashSet::new();
         for gen_list in &gen_locks {
             for obj in gen_list.iter() {
-                if retired.contains(&obj.gc_owner()) {
+                if retired.binary_search(&obj.gc_owner()).is_ok() {
                     obj.set_gc_owner(GC_NO_OWNER);
                 }
                 if obj.strong_count() > 0 && is_owned_by(obj, owner) {
@@ -553,11 +560,15 @@ impl GcState {
         // is where adoption finishes and the tags stop being tracked.
         if generation == 2 && !retired.is_empty() {
             for obj in self.permanent_list.read().iter() {
-                if retired.contains(&obj.gc_owner()) {
+                if retired.binary_search(&obj.gc_owner()).is_ok() {
                     obj.set_gc_owner(GC_NO_OWNER);
                 }
             }
-            self.retired.lock().retain(|tag| !retired.contains(tag));
+            // Only the tags this scan saw: one retired while it ran still has
+            // objects nobody has adopted.
+            self.retired
+                .lock()
+                .retain(|tag| retired.binary_search(tag).is_err());
         }
 
         if collecting.is_empty() {
@@ -1281,7 +1292,8 @@ impl Drop for GcInterpreterState {
     fn drop(&mut self) {
         // Objects this interpreter tracked can outlive it (another interpreter
         // may still hold one). Clearing the tag hands them to every collection
-        // instead of stranding them, and frees the tag for reuse.
+        // instead of stranding them. The tag itself is not handed back: it stays
+        // retired so that a later interpreter cannot inherit these objects.
         gc_state().retire_owner(self.owner);
     }
 }

diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs
@@ -449,7 +449,10 @@ impl Interpreter {
         self.global_state.whence
     }
 
-    /// Whether this is the process main interpreter.
+    /// Whether this is a top-level interpreter rather than a subinterpreter.
+    ///
+    /// Every top-level interpreter answers `true`; for *the* process main, use
+    /// [`Interpreter::is_process_main`].
     #[inline]
     #[must_use]
     pub fn is_main(&self) -> bool {
@@ -1358,11 +1361,12 @@ mod tests {
         let sub = main.create_subinterpreter();
         let id = sub.id();
 
-        let before = runtime::owned_interpreter_count();
         assert_eq!(runtime::store_owned_interpreter(sub), id);
         assert!(runtime::is_owned_interpreter(id));
         assert!(runtime::lookup_interpreter(id).is_some());
-        assert_eq!(runtime::owned_interpreter_count(), before + 1);
+        // The owned table is process-global and other tests store into it in
+        // parallel, so only this entry's own membership is deterministic.
+        assert!(runtime::owned_interpreter_count() >= 1);
 
         // Reclaiming removes ownership but keeps the interpreter alive while the
         // returned handle is held.

diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs
@@ -749,7 +749,9 @@ pub struct PyGlobalState {
     pub interpreter_id: i64,
     /// How this interpreter was created.
     pub whence: runtime::InterpreterWhence,
-    /// True only for the process main interpreter.
+    /// True for every top-level (non-sub) interpreter, each of which keeps its
+    /// own signal and main-thread bookkeeping. Only the first one registered
+    /// becomes *the* process main — see [`runtime::main_interpreter_id`].
     pub is_main: bool,
     pub config: PyConfig,
     pub module_defs: BTreeMap<&'static str, &'static builtins::PyModuleDef>,