From e1e358624d3df3a4e4d1a50d677a6675947fc0d2 Mon Sep 17 00:00:00 2001
From: Douglas Katzman <dougk@google.com>
Date: Sun, 19 Mar 2017 11:38:39 -0400
Subject: [PATCH] Hoist tests from scan_weak_pointers() into
 scav_weak_pointer()

Of particularly high relevance is the already-broken wp.
This keeps the scan list shorter, often by a lot.
---
 src/runtime/gc-common.c   | 18 +++++++++---------
 src/runtime/gc-internal.h | 14 ++++++++++++++
 src/runtime/gencgc.c      |  2 +-
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/runtime/gc-common.c b/src/runtime/gc-common.c
index f66a25363..02d63c015 100644
--- a/src/runtime/gc-common.c
+++ b/src/runtime/gc-common.c
@@ -907,9 +907,6 @@ static lispobj
 trans_weak_pointer(lispobj object)
 {
     lispobj copy;
-#ifndef LISP_FEATURE_GENCGC
-    struct weak_pointer *wp;
-#endif
     gc_dcheck(lowtag_of(object) == OTHER_POINTER_LOWTAG);
 
 #if defined(DEBUG_WEAK)
@@ -921,12 +918,14 @@ trans_weak_pointer(lispobj object)
 
     copy = copy_object(object, WEAK_POINTER_NWORDS);
 #ifndef LISP_FEATURE_GENCGC
-    wp = (struct weak_pointer *) native_pointer(copy);
+    struct weak_pointer *wp = (struct weak_pointer *) native_pointer(copy);
 
     gc_dcheck(widetag_of(wp->header)==WEAK_POINTER_WIDETAG);
     /* Push the weak pointer onto the list of weak pointers. */
-    wp->next = (struct weak_pointer *)LOW_WORD(weak_pointers);
-    weak_pointers = wp;
+    if (weak_pointer_breakable_p(wp)) {
+        wp->next = (struct weak_pointer *)LOW_WORD(weak_pointers);
+        weak_pointers = wp;
+    }
 #endif
     return copy;
 }
@@ -942,7 +941,6 @@ void scan_weak_pointers(void)
 {
     struct weak_pointer *wp, *next_wp;
     for (wp = weak_pointers, next_wp = NULL; wp != NULL; wp = next_wp) {
-        lispobj value = wp->value;
         lispobj *first_pointer;
         gc_assert(widetag_of(wp->header)==WEAK_POINTER_WIDETAG);
 
@@ -951,8 +949,8 @@ void scan_weak_pointers(void)
         if (next_wp == wp) /* gencgc uses a ref to self for end of list */
             next_wp = NULL;
 
-        if (!is_lisp_pointer(value))
-            continue;
+        lispobj value = wp->value;
+        gc_assert(is_lisp_pointer(value));
 
         /* Now, we need to check whether the object has been forwarded. If
          * it has been, the weak pointer is still good and needs to be
@@ -977,6 +975,8 @@ void scan_weak_pointers(void)
                 wp->broken = T;
         }
 #endif
+        else
+            lose("unbreakable pointer %p", wp);
     }
 }
 
diff --git a/src/runtime/gc-internal.h b/src/runtime/gc-internal.h
index 4b5db6f97..23525e1fa 100644
--- a/src/runtime/gc-internal.h
+++ b/src/runtime/gc-internal.h
@@ -18,6 +18,7 @@
 
 #include "genesis/code.h"
 #include "genesis/simple-fun.h"
+#include "genesis/weak-pointer.h"
 #include "thread.h"
 #include "interr.h"
 
@@ -329,4 +330,17 @@ static inline boolean immobile_filler_p(lispobj* obj) {
 
 #endif /* immobile space */
 
+static inline boolean weak_pointer_breakable_p(struct weak_pointer *wp)
+{
+    lispobj pointee;
+    return wp->broken != T &&
+        is_lisp_pointer(pointee = wp->value) &&
+        (from_space_p(pointee)
+#ifdef LISP_FEATURE_IMMOBILE_SPACE
+         || (immobile_space_p(pointee) &&
+             immobile_obj_gen_bits(native_pointer(pointee)) == from_space)
+#endif
+            );
+}
+
 #endif /* _GC_INTERNAL_H_ */
diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
index f7d0cd4f5..480dde14d 100644
--- a/src/runtime/gencgc.c
+++ b/src/runtime/gencgc.c
@@ -1938,7 +1938,7 @@ scav_weak_pointer(lispobj *where, lispobj object)
      */
     struct weak_pointer * wp = (struct weak_pointer*)where;
 
-    if (NULL == wp->next) {
+    if (NULL == wp->next && weak_pointer_breakable_p(wp)) {
         wp->next = weak_pointers;
         weak_pointers = wp;
         if (NULL == wp->next)
-- 
2.11.4.GIT