GitLab

at 2024-03-01T08:15:23-08:00

Revert "Update some comments; slightly modify scan_static_vectors to use pop"

This reverts commit 1f8a6328ba47804660e1811a8227522d86be0259.

Revert "Clear static vector visited bit when scavenging a weak pointer to it"

This reverts commit 0bcba23ea0319d26552af1766273a61a5fd6c0cd.

Revert "Remove unmark_static_vectors_in_use and inuse_static_vector_list"

This reverts commit 081dcba913e565de7a0fbcf0ad68113fec78c1e7.

Revert "Don't need inuse list anymore"

This reverts commit 8ae61cf30fe5e2f09df91e90f814adf1c75fef20.

Revert "First cut at clearing the mark during phase 1"

This reverts commit c3be06f3c38118aef6137dae1df956fdf689e93a.

 static int (*sizetab[256]) (lispobj * where);

 static struct weak_pointer *weak_pointers;

+static struct weak_pointer *inuse_static_vector_list;

 static struct scavenger_hook *scavenger_hooks = (struct scavenger_hook *) NIL;

 /* Like (ceiling x y), but y is constrained to be a power of two */

         int static_p;

         if (debug_static_array_p) {

-            fprintf(stderr, "Possible static vector at %p.  header = 0x%08lx\n",

+            fprintf(stderr, "Possible static vector at %p.  header = 0x%lx\n",

                     ptr, (unsigned long) header);

         }

              * setting the MSB of the header.  And clear out any

              * possible visited bit.

              */

-            *ptr = (header | STATIC_VECTOR_MARK_BIT);

+            *ptr = (header | STATIC_VECTOR_MARK_BIT) & ~STATIC_VECTOR_VISITED_BIT;

             if (debug_static_array_p) {

-                fprintf(stderr, "Scavenged static vector @%p, header = 0x%08lx\n",

+                fprintf(stderr, "Scavenged static vector @%p, header = 0x%lx\n",

                         ptr, (unsigned long) header);

             }

         }

     struct weak_pointer *this_wp = (struct weak_pointer *) where;

     if (this_wp->mark_bit == NIL) {

-        lispobj *header = (lispobj *) PTR(this_wp->value);

-        if (maybe_static_array_p(*header)) {

-            printf("Scavenge wp %p to static array %p header 0x%08lx\n",

-                   this_wp, (lispobj *) this_wp->value, *header);

-

-            *header &= ~STATIC_VECTOR_VISITED_BIT;

-        }

-

 	this_wp->mark_bit = T;

 	this_wp->next = weak_pointers;

 	weak_pointers = this_wp;

 }

 /*

- * Phase 2: Process the list of weak pointers to unused static

- * vectors.  We find the unique static vectors and add each

- * corresponding weak pointer to freeable_list.  For the duplicates,

- * the weak pointer is broken.

+ * Phase 2: Find the weak pointers to static vectors that are in use

+ * and not in use.  The vectors that are in use are added to

+ * inuse_list.  Those that are not in use are added to freeable_list.

+ * Only unique vectors are added to freeable list; a duplicate has its

+ * weak pointer to it broken.

  */

 static void

 scan_static_vectors_2(struct weak_pointer *static_vector_list,

-                      struct weak_pointer **freeable_list)

+                      struct weak_pointer **freeable_list,

+                      struct weak_pointer **inuse_list)

 {

     DPRINTF(debug_static_array_p,

             (stdout, "Phase 2: Find unused and unused static vectors\n"));

                 (stdout, "  wp %p value %p header 0x%08lx\n",

                  wp, (lispobj *) wp->value, *header));

-        /*

-         * If we haven't seen this vector before, set the visited flag

-         * and add it to freeable_list.  If we have visited this

-         * vector before, break the weak pointer.

-         */

-        if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) {

+        if ((*header & STATIC_VECTOR_MARK_BIT) != 0) {

+            /*

+             * Static vector is in use.  Add this to the in-use list.

+             */

             DPRINTF(debug_static_array_p,

-                    (stdout, "    Visit unused vector, add to freeable list\n"));

+                    (stdout, "    In-use vector; add to in-use list\n"));

-            *header |= STATIC_VECTOR_VISITED_BIT;

-            push_weak_pointer(wp, freeable_list);

+            push_weak_pointer(wp, inuse_list);

         } else {

-            DPRINTF(debug_static_array_p,

-                    (stdout, "    Already visited unused vector; break weak pointer\n"));

+            /*

+             * Static vector not in use.  If we haven't seen this

+             * vector before, set the visited flag and add it to

+             * freeable_list.  If we have visited this vector before,

+             * break the weak pointer.

+             */

+            if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) {

+                DPRINTF(debug_static_array_p,

+                        (stdout, "    Visit unused vector, add to freeable list\n"));

-            wp->value = NIL;

-            wp->broken = T;

+                *header |= STATIC_VECTOR_VISITED_BIT;

+                push_weak_pointer(wp, freeable_list);

+            } else {

+                DPRINTF(debug_static_array_p,

+                        (stdout, "    Already visited unused vector; break weak pointer\n"));

+

+                wp->value = NIL;

+                wp->broken = T;

+            }

         }

     }

         lispobj *header = (lispobj *) PTR(wp->value);

         lispobj *static_array = (lispobj *) PTR(wp->value);

+        /*

+         * Invariant: weak pointer must not be broken

+         */

+        gc_assert(wp->broken == NIL);

+

         DPRINTF(debug_static_array_p,

                 (stdout, "  wp %p value %p header 0x%08lx\n",

                  wp, (lispobj*) wp->value, *header));

         /*

-         * Invariants: weak pointer must not be broken and the mark

-         * bit must be clear.

+         * Invariant: Mark bit must be clear

          */

-        gc_assert(wp->broken == NIL);

         gc_assert(((*header & STATIC_VECTOR_MARK_BIT) == 0));

         DPRINTF(debug_static_array_p,

             (stdout, "Phase 3 done\n"));

 }

+/*

+ * Unmark all the vectors in inuse_list.  This needs to be called at

+ * the end of GC to unmark any live static vectors so that for the

+ * next GC we can tell if the static vector is used or not.

+ * Otherwise, the vectors will always look as if they're in use

+ * because the mark bit is never changed.

+ */

+static void

+unmark_static_vectors_in_use(struct weak_pointer *inuse_list)

+{

+    struct weak_pointer *wp;

+

+    DPRINTF(debug_static_array_p,

+            (stdout, "Phase 4: unmark static vectors\n"));

+

+    for (wp = inuse_list; wp; wp = wp->next) {

+        lispobj *header = (lispobj *) PTR(wp->value);

+        /*

+         * Invariant: the weak pointer must not be broken.

+         *

+         * Note that we can't assert that the static vector is marked

+         * because we can have more than one weak pointer to the same

+         * static vector.

+         */

+        gc_assert(wp->broken == NIL);

+        

+        DPRINTF(debug_static_array_p,

+                (stdout, "  wp %p value %p broken %d header 0x%08lx\n",

+                 wp, (lispobj*) wp->value, wp->broken == T, *header));

+

+        /* Only clear if we haven't already */

+        if ((*header & STATIC_VECTOR_MARK_BIT) != 0) {

+            DPRINTF(debug_static_array_p,

+                    (stdout, "    Clearing mark bit\n"));

+

+            *header &= ~STATIC_VECTOR_MARK_BIT;

+        }

+    }

+

+    DPRINTF(debug_static_array_p,

+            (stdout, "Phase 4 done\n"));

+}

+

 static void

 scan_static_vectors(struct weak_pointer *static_vector_list)

 {

     struct weak_pointer *freeable_list = NULL;

     /*

-     * For each weak pointer, either break it, or add it to the

-     * freeable list because it points to a unique static vector.

+     * For each weak pointer, add it either the inuse list or the

+     * freeable list.

      */

-    scan_static_vectors_2(static_vector_list, &freeable_list);

+    inuse_static_vector_list = NULL;

+    scan_static_vectors_2(static_vector_list, &freeable_list, &inuse_static_vector_list);

     /* Free the unused unique static vectors. */

     scan_static_vectors_3(freeable_list);

      *

      * Also find any weak pointers to static vectors.  This

      * destructively modifies the next slot of the weak pointer to

-     * chain all the weak pointers to unused static vectors together.

+     * chain all the weak pointers to static vectors together.

      */

     DPRINTF(debug_static_array_p,

             (stdout, "Phase 1: Process weak pointers\n"));

-    while (weak_pointers) {

-	lispobj value;

-	lispobj *first_pointer;

-

-        wp = pop_weak_pointer(&weak_pointers);

-	value = wp->value;

-	first_pointer = (lispobj *) PTR(value);

+    while (wp) {

+        struct weak_pointer *next = wp->next;

+	lispobj value = wp->value;

+	lispobj *first_pointer = (lispobj *) PTR(value);

 	wp->mark_bit = NIL;

 	if (Pointerp(value)) {

                 }

             } else {

                 /* The value may be a static vector */

-                lispobj *header = (lispobj *) PTR(value);

-

-                if (maybe_static_array_p(*header)) {

-                    if (*header & STATIC_VECTOR_MARK_BIT) {

-                        DPRINTF(debug_static_array_p,

-                                (stdout, "  Update status bits for live vector: wp %p value %p header 0x%08lx\n",

-                                 wp, (lispobj *) wp->value, *header));

-                        *header = (*header & ~STATIC_VECTOR_MARK_BIT) | STATIC_VECTOR_VISITED_BIT;

-                    } else if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) {

-                        /* Only add the vector if it is unmarked and has not been visited */

-                        DPRINTF(debug_static_array_p,

-                                (stdout, "  Add static vector:  wp %p value %p header 0x%08lx\n",

-                                 wp, (lispobj *) wp->value, *header));

-

-                        push_weak_pointer(wp, &static_vector_list);

-                    }

+                lispobj header = *(lispobj *) PTR(value);

+

+                if (maybe_static_array_p(header)) {

+

+                    DPRINTF(debug_static_array_p,

+                            (stdout, "  Add static vector:  wp %p value %p header 0x%08lx\n",

+                             wp, (lispobj *) wp->value, header));

+

+                    push_weak_pointer(wp, &static_vector_list);

                 } else {

                     DPRINTF(debug_static_array_p,

                             (stdout, "  Skip: wp %p value %p header 0x%08lx\n",

-                             wp, (lispobj *) wp->value, *header));

+                             wp, (lispobj *) wp->value, header));

                 }

             }

         }

+        wp = next;

     }

     scan_static_vectors(static_vector_list);

 	}

 	scavenger_hooks = (struct scavenger_hook *) NIL;

     }

+

+    /*

+     * Unmark any live static vectors.  This needs to be done at the

+     * very end when all GCs are done, lest we accidentally free a

+     * static vector that was actually in use.

+     */

+    unmark_static_vectors_in_use(inuse_static_vector_list);

 }

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl

Commits:

1 changed file:

Changes: