From: Alastair Bridgewater <nyef@kana.lisphacker.com>
Date: Tue, 31 Dec 2013 14:07:32 +0000 (-0500)
Subject: gencgc: Extract go/no-go decision logic from preserve_pointer().
X-Git-Url: http://repo.macrolet.net/gitweb/?p=sbcl.git;a=commitdiff_plain;h=8f79e6459a0e8fdd33c81a66f7e4adfa13f25005

gencgc: Extract go/no-go decision logic from preserve_pointer().

  * Approximately 1/pi of preserve_pointer() is involved in
verifying that the candidate pointer is valid as a conservative
root.  This is easily extractable logic that acts as a predicate,
so box it up as one, calling it as a guard clause from where it
originally stood.
---

diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
index 77ec797..1d48cc3 100644
--- a/src/runtime/gencgc.c
+++ b/src/runtime/gencgc.c
@@ -2074,6 +2074,46 @@ possibly_valid_dynamic_space_pointer(lispobj *pointer)
 
 #endif  // defined(LISP_FEATURE_X86) || defined(LISP_FEATURE_X86_64)
 
+static int
+valid_conservative_root_p(void *addr, page_index_t addr_page_index)
+{
+    /* quick check 1: Address is quite likely to have been invalid. */
+    if ((addr_page_index == -1)
+        || page_free_p(addr_page_index)
+        || (page_table[addr_page_index].bytes_used == 0)
+        || (page_table[addr_page_index].gen != from_space)
+        /* Skip if already marked dont_move. */
+        || (page_table[addr_page_index].dont_move != 0))
+        return 0;
+    gc_assert(!(page_table[addr_page_index].allocated&OPEN_REGION_PAGE_FLAG));
+
+    /* quick check 2: Check the offset within the page.
+     *
+     */
+    if (((uword_t)addr & (GENCGC_CARD_BYTES - 1)) >
+        page_table[addr_page_index].bytes_used)
+        return 0;
+
+    /* Filter out anything which can't be a pointer to a Lisp object
+     * (or, as a special case which also requires dont_move, a return
+     * address referring to something in a CodeObject). This is
+     * expensive but important, since it vastly reduces the
+     * probability that random garbage will be bogusly interpreted as
+     * a pointer which prevents a page from moving.
+     *
+     * This only needs to happen on x86oids, where this is used for
+     * conservative roots.  Non-x86oid systems only ever call this
+     * function on known-valid lisp objects. */
+#if defined(LISP_FEATURE_X86) || defined(LISP_FEATURE_X86_64)
+    if (!(code_page_p(addr_page_index)
+          || (is_lisp_pointer((lispobj)addr) &&
+              possibly_valid_dynamic_space_pointer(addr))))
+        return 0;
+#endif
+
+    return 1;
+}
+
 /* Adjust large bignum and vector objects. This will adjust the
  * allocated region if the size has shrunk, and move unboxed objects
  * into unboxed pages. The pages are not promoted here, and the
@@ -2261,43 +2301,13 @@ preserve_pointer(void *addr)
     page_index_t i;
     unsigned int region_allocation;
 
-    /* quick check 1: Address is quite likely to have been invalid. */
-    if ((addr_page_index == -1)
-        || page_free_p(addr_page_index)
-        || (page_table[addr_page_index].bytes_used == 0)
-        || (page_table[addr_page_index].gen != from_space)
-        /* Skip if already marked dont_move. */
-        || (page_table[addr_page_index].dont_move != 0))
+    if (!valid_conservative_root_p(addr, addr_page_index))
         return;
-    gc_assert(!(page_table[addr_page_index].allocated&OPEN_REGION_PAGE_FLAG));
+
     /* (Now that we know that addr_page_index is in range, it's
      * safe to index into page_table[] with it.) */
     region_allocation = page_table[addr_page_index].allocated;
 
-    /* quick check 2: Check the offset within the page.
-     *
-     */
-    if (((uword_t)addr & (GENCGC_CARD_BYTES - 1)) >
-        page_table[addr_page_index].bytes_used)
-        return;
-
-    /* Filter out anything which can't be a pointer to a Lisp object
-     * (or, as a special case which also requires dont_move, a return
-     * address referring to something in a CodeObject). This is
-     * expensive but important, since it vastly reduces the
-     * probability that random garbage will be bogusly interpreted as
-     * a pointer which prevents a page from moving.
-     *
-     * This only needs to happen on x86oids, where this is used for
-     * conservative roots.  Non-x86oid systems only ever call this
-     * function on known-valid lisp objects. */
-#if defined(LISP_FEATURE_X86) || defined(LISP_FEATURE_X86_64)
-    if (!(code_page_p(addr_page_index)
-          || (is_lisp_pointer((lispobj)addr) &&
-              possibly_valid_dynamic_space_pointer(addr))))
-        return;
-#endif
-
     /* Find the beginning of the region.  Note that there may be
      * objects in the region preceding the one that we were passed a
      * pointer to: if this is the case, we will write-protect all the