February 2024 - cmucl-cvs - mailman3.common-lisp.net

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Fix typo and init inuse list in scan_static_vectors
by Raymond Toy (＠rtoy) 24 Feb '24

24 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: f1621d3a by Raymond Toy at 2024-02-24T11:38:11-08:00 Fix typo and init inuse list in scan_static_vectors - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5597,6 +5597,7 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * For each weak pointer, add it either the inuse list or the * 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. */ @@ -8249,8 +8250,6 @@ collect_garbage(unsigned last_gen) int gen_to_wp; int i; - inuse_static_vectors_lisit = NULL; - boxed_region.free_pointer = (void *) get_current_region_free(); /* Check last_gen */ View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/f1621d3a52aeff4185614b3… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/f1621d3a52aeff4185614b3… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Unmark static vectors at the end of GC
by Raymond Toy (＠rtoy) 24 Feb '24

24 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 6890f1a5 by Raymond Toy at 2024-02-24T10:07:39-08:00 Unmark static vectors at the end of GC Before, we were unmarking live static vectors at the end of `scan_static_vectors`. But apparently this is not quite right because when doing `(gc :full t)` multiple times, we clear the mark bit to static vectors that are actually still alive and that, of course, causes havoc. Instead, unmark the vectors at the end of collect_garbage when we've finished all GCs. This kind of matches how we were originally unmarking the static vectors in a post-GC hook. With this change when running our weak-pointer test, `(gc :full t)` can be called many times without causing issues. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -2130,6 +2130,7 @@ static lispobj(*transother[256]) (lispobj object); 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 */ @@ -5496,6 +5497,9 @@ scan_static_vectors_2(struct weak_pointer *static_vector_list, } } } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 2 done\n")); } /* @@ -5535,13 +5539,20 @@ scan_static_vectors_3(struct weak_pointer *freeable_list) free(static_array); } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 3 done\n")); } /* - * Unmark all the vectors in inuse_list + * 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 -scan_static_vectors_4(struct weak_pointer *inuse_list) +unmark_static_vectors_in_use(struct weak_pointer *inuse_list) { struct weak_pointer *wp; @@ -5571,6 +5582,9 @@ scan_static_vectors_4(struct weak_pointer *inuse_list) *header &= ~STATIC_VECTOR_MARK_BIT; } } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 4 done\n")); } static void @@ -5579,20 +5593,14 @@ scan_static_vectors(struct weak_pointer *static_vector_list) /* List of weak pointers to static vectors that can be freed. */ struct weak_pointer *freeable_list = NULL; - /* List of weak pointers to static vectors that are in in use. */ - struct weak_pointer *inuse_list = NULL; - /* * For each weak pointer, add it either the inuse list or the * freeable list. */ - scan_static_vectors_2(static_vector_list, &freeable_list, &inuse_list); + 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); - - /* Unmark all the static vectors that are still alive. */ - scan_static_vectors_4(inuse_list); } void @@ -8241,6 +8249,8 @@ collect_garbage(unsigned last_gen) int gen_to_wp; int i; + inuse_static_vectors_lisit = NULL; + boxed_region.free_pointer = (void *) get_current_region_free(); /* Check last_gen */ @@ -8380,6 +8390,13 @@ collect_garbage(unsigned last_gen) } 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); } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/6890f1a5498c97f4d896738… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/6890f1a5498c97f4d896738… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Use push in scan_weak_pointers to add a static vector
by Raymond Toy (＠rtoy) 23 Feb '24

23 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: a99966c8 by Raymond Toy at 2024-02-23T15:39:36-08:00 Use push in scan_weak_pointers to add a static vector - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5636,8 +5636,7 @@ scan_weak_pointers(void) (stdout, " Add static vector: wp %p value %p header 0x%08lx\n", wp, (lispobj *) wp->value, header)); - wp->next = static_vector_list; - static_vector_list = wp; + push(wp, &static_vector_list); } else { DPRINTF(debug_static_array_p, (stdout, " Skip: wp %p value %p header 0x%08lx\n", View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/a99966c892cd8d9d2053080… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/a99966c892cd8d9d2053080… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Move phases of scanning static vectors into separate functions.
by Raymond Toy (＠rtoy) 23 Feb '24

23 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: afb81c84 by Raymond Toy at 2024-02-23T14:41:27-08:00 Move phases of scanning static vectors into separate functions. Make `scan_static_vectors` a bit easier to read by putting each phase into its own function. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5439,85 +5439,84 @@ push(struct weak_pointer* wp, struct weak_pointer **list) *list = wp; } +/* + * 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(struct weak_pointer *static_vector_list) +scan_static_vectors_2(struct weak_pointer *static_vector_list, + struct weak_pointer **freeable_list, + struct weak_pointer **inuse_list) { - /* List of weak pointers to static vectors that can be freed. */ - struct weak_pointer *freeable_list = NULL; - - /* List of weak pointers to static vectors that are in in use. */ - struct weak_pointer *inuse_list = NULL; - struct weak_pointer *wp; - DPRINTF(debug_static_array_p, (stdout, "Phase 2: Find unused and unused static vectors\n")); - /* - * static_vector_list points to all weak pointers to static - * vectors. If the static vector is in use, add it to the in-use - * list. If the static vector is not in use, and has not been - * visited, mark it as visited and add it to the freeable list. - * If it has been visited, just break the weak pointer. - */ while (static_vector_list) { - struct weak_pointer *this_wp; + struct weak_pointer *wp; lispobj *header; /* Pop weak pointer from the list */ - this_wp = pop(&static_vector_list); - header = (lispobj *) PTR(this_wp->value); + wp = pop(&static_vector_list); + header = (lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, (stdout, " wp %p value %p header 0x%08lx\n", - this_wp, (lispobj *) this_wp->value, *header)); + wp, (lispobj *) wp->value, *header)); - /* - * If the static vector is unused (mark bit clear) and if we - * haven't seen this vector before, set the visited flag. If - * we have visited this vector before, break the weak pointer. - */ if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { /* * Static vector is in use. Add this to the in-use list. */ - printf(" In-use vector; add to in-use list\n"); - push(this_wp, &inuse_list); + DPRINTF(debug_static_array_p, + (stdout, " In-use vector; add to in-use list\n")); + + push(wp, inuse_list); } else { - /* Unused static vector */ + /* + * 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")); *header |= STATIC_VECTOR_VISITED_BIT; - - /* - * Add this to the freeable list because it's a static - * vector that we can free. - */ - push(this_wp, &freeable_list); + push(wp, freeable_list); } else { DPRINTF(debug_static_array_p, (stdout, " Already visited unused vector; break weak pointer\n")); - this_wp->value = NIL; - this_wp->broken = T; + wp->value = NIL; + wp->broken = T; } } } +} + +/* + * Free all the unused static vectors in freeable_list. + */ +static void +scan_static_vectors_3(struct weak_pointer *freeable_list) +{ + struct weak_pointer *wp; DPRINTF(debug_static_array_p, (stdout, "Phase 3: Free static vectors\n")); - /* - * freeable_list now contains weak pointers to unique static - * vectors that are not in use. Break the weak pointer and free - * the static vector. - */ for (wp = freeable_list; wp; wp = wp->next) { - /* Invariant: weak pointer must not be broken */ - gc_assert(wp->broken == NIL); - 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", @@ -5527,8 +5526,7 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * Invariant: Mark bit must be clear */ gc_assert(((*header & STATIC_VECTOR_MARK_BIT) == 0)); - - lispobj *static_array = (lispobj *) PTR(wp->value); + DPRINTF(debug_static_array_p, (stdout, " Free static vector\n")); @@ -5537,14 +5535,19 @@ scan_static_vectors(struct weak_pointer *static_vector_list) free(static_array); } +} + +/* + * Unmark all the vectors in inuse_list + */ +static void +scan_static_vectors_4(struct weak_pointer *inuse_list) +{ + struct weak_pointer *wp; DPRINTF(debug_static_array_p, (stdout, "Phase 4: unmark static vectors\n")); - /* - * At this point, inuse_list is a list of weak pointers to static - * vectors that are still in use. Clear the mark bit. - */ for (wp = inuse_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); /* @@ -5560,6 +5563,7 @@ scan_static_vectors(struct weak_pointer *static_vector_list) (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")); @@ -5569,6 +5573,28 @@ scan_static_vectors(struct weak_pointer *static_vector_list) } } +static void +scan_static_vectors(struct weak_pointer *static_vector_list) +{ + /* List of weak pointers to static vectors that can be freed. */ + struct weak_pointer *freeable_list = NULL; + + /* List of weak pointers to static vectors that are in in use. */ + struct weak_pointer *inuse_list = NULL; + + /* + * For each weak pointer, add it either the inuse list or the + * freeable list. + */ + scan_static_vectors_2(static_vector_list, &freeable_list, &inuse_list); + + /* Free the unused unique static vectors. */ + scan_static_vectors_3(freeable_list); + + /* Unmark all the static vectors that are still alive. */ + scan_static_vectors_4(inuse_list); +} + void scan_weak_pointers(void) { View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/afb81c84f83b2ca4b976b78… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/afb81c84f83b2ca4b976b78… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Remove unneeded pointers during processing.
by Raymond Toy (＠rtoy) 23 Feb '24

23 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: e400cea3 by Raymond Toy at 2024-02-23T14:04:08-08:00 Remove unneeded pointers during processing. Add `pop` and `push` functions to handle the lists we need to keep track of things. In phase 2, live vectors are added to the in-use list. For unused vectors, the first visit adds them to the freeable list. Subsequent visits to this static vector break the weak pointer. In phase 3, process the freeable list by freeing the vector and breaking the corresponding weak pointer to the vector. In phase 4, process the in-use list and unmark the vectors so the next GC can tell if the vector is in use or not. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5417,113 +5417,154 @@ size_weak_pointer(lispobj * where) return WEAK_POINTER_NWORDS; } +static inline struct weak_pointer * +pop(struct weak_pointer **list) +{ + /* + * Pop the first element from the list `list`, updating `list`, like CL POP. + */ + struct weak_pointer *wp = *list; + *list = (*list)->next; + + return wp; +} + +static inline void +push(struct weak_pointer* wp, struct weak_pointer **list) +{ + /* + * Push wp to the head of the list, updating list, like CL PUSH. + */ + wp->next = *list; + *list = wp; +} static void scan_static_vectors(struct weak_pointer *static_vector_list) { + /* List of weak pointers to static vectors that can be freed. */ + struct weak_pointer *freeable_list = NULL; + + /* List of weak pointers to static vectors that are in in use. */ + struct weak_pointer *inuse_list = NULL; struct weak_pointer *wp; DPRINTF(debug_static_array_p, - (stdout, "Phase 2: Visit unused static vectors\n")); + (stdout, "Phase 2: Find unused and unused static vectors\n")); /* - * static_vector_list now points to all weak pointers to static - * vectors. For unmarked (unused) static vectors, set another bit - * in the header to say we've visited it. If we've already - * visited the static vector, break the weak pointer. + * static_vector_list points to all weak pointers to static + * vectors. If the static vector is in use, add it to the in-use + * list. If the static vector is not in use, and has not been + * visited, mark it as visited and add it to the freeable list. + * If it has been visited, just break the weak pointer. */ - for (wp = static_vector_list; wp; wp = wp->next) { - lispobj *header = (lispobj *) PTR(wp->value); + while (static_vector_list) { + struct weak_pointer *this_wp; + lispobj *header; + + /* Pop weak pointer from the list */ + this_wp = pop(&static_vector_list); + header = (lispobj *) PTR(this_wp->value); DPRINTF(debug_static_array_p, (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj *) wp->value, *header)); + this_wp, (lispobj *) this_wp->value, *header)); /* * If the static vector is unused (mark bit clear) and if we * haven't seen this vector before, set the visited flag. If * we have visited this vector before, break the weak pointer. */ - if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { + if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { + /* + * Static vector is in use. Add this to the in-use list. + */ + printf(" In-use vector; add to in-use list\n"); + push(this_wp, &inuse_list); + } else { /* Unused static vector */ if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) { - DPRINTF(debug_static_array_p, (stdout, " Mark vector\n")); + DPRINTF(debug_static_array_p, + (stdout, " Visit unused vector, add to freeable list\n")); *header |= STATIC_VECTOR_VISITED_BIT; + + /* + * Add this to the freeable list because it's a static + * vector that we can free. + */ + push(this_wp, &freeable_list); } else { DPRINTF(debug_static_array_p, - (stdout, " Break weak pointer %p\n", wp)); + (stdout, " Already visited unused vector; break weak pointer\n")); - wp->value = NIL; - wp->broken = T; + this_wp->value = NIL; + this_wp->broken = T; } } } - DPRINTF(debug_static_array_p, (stdout, "Phase 3: Free static vectors\n")); /* - * static_vector_list now contains either broken weak pointers or - * weak pointers to static arrays (whether alive or not). - * - * Free up space. Go through static_vector_list and for each weak - * pointer that hasn't been broken and is an unused static array, - * free the static vector. Also break the weak pointer too, since - * the space has been freed. + * freeable_list now contains weak pointers to unique static + * vectors that are not in use. Break the weak pointer and free + * the static vector. */ - for (wp = static_vector_list; wp; wp = wp->next) { - /* Skip over broken weak pointers */ - if (wp->broken == NIL) { - lispobj *header = (lispobj *) PTR(wp->value); + for (wp = freeable_list; wp; wp = wp->next) { + /* Invariant: weak pointer must not be broken */ + gc_assert(wp->broken == NIL); + + lispobj *header = (lispobj *) PTR(wp->value); - DPRINTF(debug_static_array_p, - (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj*) wp->value, *header)); + DPRINTF(debug_static_array_p, + (stdout, " wp %p value %p header 0x%08lx\n", + wp, (lispobj*) wp->value, *header)); - /* - * Only free the arrays where the mark bit is clear. - */ - if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { - lispobj *static_array = (lispobj *) PTR(wp->value); - DPRINTF(debug_static_array_p, - (stdout, " Free static vector\n")); + /* + * Invariant: Mark bit must be clear + */ + gc_assert(((*header & STATIC_VECTOR_MARK_BIT) == 0)); + + lispobj *static_array = (lispobj *) PTR(wp->value); + DPRINTF(debug_static_array_p, + (stdout, " Free static vector\n")); - wp->value = NIL; - wp->broken = T; + wp->value = NIL; + wp->broken = T; - free(static_array); - } - } + free(static_array); } - DPRINTF(debug_static_array_p, (stdout, "Phase 4: unmark static vectors\n")); /* - * At this point, static_vector_list contains weak pointers that - * have been broken or weak pointres to live static vectors. Go - * through all the weak pointers and if it hasn't been broken and - * if the mark bit of the static vector is set, then clear the - * mark bit . + * At this point, inuse_list is a list of weak pointers to static + * vectors that are still in use. Clear the mark bit. */ - for (wp = static_vector_list; wp; wp = wp->next) { - /* Skip over broken weak pointers */ - if (wp->broken == NIL) { - lispobj *header = (lispobj *) PTR(wp->value); + 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)); + if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { DPRINTF(debug_static_array_p, - (stdout, " wp %p value %p broken %d header 0x%08lx\n", - wp, (lispobj*) wp->value, wp->broken == T, *header)); + (stdout, " Clearing mark bit\n")); - if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { - DPRINTF(debug_static_array_p, - (stdout, " Clearing mark bit\n")); - - *header &= ~STATIC_VECTOR_MARK_BIT; - } + *header &= ~STATIC_VECTOR_MARK_BIT; } } } @@ -5543,7 +5584,7 @@ scan_weak_pointers(void) * chain all the weak pointers to static vectors together. */ DPRINTF(debug_static_array_p, - (stdout, "Phase 0: Process weak pointers\n")); + (stdout, "Phase 1: Process weak pointers\n")); while (wp) { struct weak_pointer *next = wp->next; View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/e400cea35e9957c96049980… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/e400cea35e9957c96049980… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Revert "Remove visited weak pointer to static vector in phase 2"
by Raymond Toy (＠rtoy) 23 Feb '24

23 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: bf9f9d5e by Raymond Toy at 2024-02-23T12:43:35-08:00 Revert "Remove visited weak pointer to static vector in phase 2" This reverts commit a9b2d0819cfdeb20badfe5a9a7a146866136ca3a. We're going to do the removal in a different way. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5421,8 +5421,7 @@ size_weak_pointer(lispobj * where) static void scan_static_vectors(struct weak_pointer *static_vector_list) { - struct weak_pointer *wp = static_vector_list; - struct weak_pointer *previous = wp; + struct weak_pointer *wp; DPRINTF(debug_static_array_p, (stdout, "Phase 2: Visit unused static vectors\n")); @@ -5433,9 +5432,7 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * in the header to say we've visited it. If we've already * visited the static vector, break the weak pointer. */ - - while (wp) { - struct weak_pointer *next = wp->next; + for (wp = static_vector_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, @@ -5459,32 +5456,8 @@ scan_static_vectors(struct weak_pointer *static_vector_list) wp->value = NIL; wp->broken = T; - /* - * Remove this weak pointer from static_vector_list; - * we're done processing it. - * - * Three cases here: - * - * 1. wp is the first in the list; update - * static_vector_list to skip over this pointer. - * - * 2. wp is the last (next = NULL); set the next - * slot of the previous pointer to NULL. - * - * 3. wp is in the middle; update the next slot of - * the previous pointer to the next value. - */ - if (wp == static_vector_list) { - static_vector_list = next; - } else if (next == NULL) { - previous->next = NULL; - } else { - previous->next = next; - } } } - previous = wp; - wp = next; } @@ -5500,31 +5473,29 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * free the static vector. Also break the weak pointer too, since * the space has been freed. */ - wp = static_vector_list; - while (wp) { - struct weak_pointer *next = wp->next; - lispobj *header = (lispobj *) PTR(wp->value); + for (wp = static_vector_list; wp; wp = wp->next) { /* Skip over broken weak pointers */ + if (wp->broken == NIL) { + lispobj *header = (lispobj *) PTR(wp->value); - DPRINTF(debug_static_array_p, - (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj*) wp->value, *header)); - - gc_assert(wp->broken == NIL); - /* - * Only free the arrays where the mark bit is clear. - */ - if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { - lispobj *static_array = (lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, - (stdout, " Free static vector\n")); + (stdout, " wp %p value %p header 0x%08lx\n", + wp, (lispobj*) wp->value, *header)); - wp->value = NIL; - wp->broken = T; + /* + * Only free the arrays where the mark bit is clear. + */ + if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { + lispobj *static_array = (lispobj *) PTR(wp->value); + DPRINTF(debug_static_array_p, + (stdout, " Free static vector\n")); + + wp->value = NIL; + wp->broken = T; - free(static_array); + free(static_array); + } } - wp = next; } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/bf9f9d5edc0427a57a5d2e1… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/bf9f9d5edc0427a57a5d2e1… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Remove visited weak pointer to static vector in phase 2
by Raymond Toy (＠rtoy) 23 Feb '24

23 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: a9b2d081 by Raymond Toy at 2024-02-22T16:07:00-08:00 Remove visited weak pointer to static vector in phase 2 In phase 2, if we've previously visited a static vector, we break the weak pointer to it. In this change, we also destructively remove the weak pointer from `static_vector_list` so that the following phase doesn't have any broken weak pointers. In destructively removing the weak pointer, we have take care of the case where the weak pointer is the first one. We also need to handle the case where the weak pointer is the last in the list. In phase 3, add an assert that the weak pointer has not been broken. The two test cases we have work as expected. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5421,7 +5421,8 @@ size_weak_pointer(lispobj * where) static void scan_static_vectors(struct weak_pointer *static_vector_list) { - struct weak_pointer *wp; + struct weak_pointer *wp = static_vector_list; + struct weak_pointer *previous = wp; DPRINTF(debug_static_array_p, (stdout, "Phase 2: Visit unused static vectors\n")); @@ -5432,7 +5433,9 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * in the header to say we've visited it. If we've already * visited the static vector, break the weak pointer. */ - for (wp = static_vector_list; wp; wp = wp->next) { + + while (wp) { + struct weak_pointer *next = wp->next; lispobj *header = (lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, @@ -5456,8 +5459,32 @@ scan_static_vectors(struct weak_pointer *static_vector_list) wp->value = NIL; wp->broken = T; + /* + * Remove this weak pointer from static_vector_list; + * we're done processing it. + * + * Three cases here: + * + * 1. wp is the first in the list; update + * static_vector_list to skip over this pointer. + * + * 2. wp is the last (next = NULL); set the next + * slot of the previous pointer to NULL. + * + * 3. wp is in the middle; update the next slot of + * the previous pointer to the next value. + */ + if (wp == static_vector_list) { + static_vector_list = next; + } else if (next == NULL) { + previous->next = NULL; + } else { + previous->next = next; + } } } + previous = wp; + wp = next; } @@ -5473,29 +5500,31 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * free the static vector. Also break the weak pointer too, since * the space has been freed. */ - for (wp = static_vector_list; wp; wp = wp->next) { + wp = static_vector_list; + while (wp) { + struct weak_pointer *next = wp->next; + lispobj *header = (lispobj *) PTR(wp->value); /* Skip over broken weak pointers */ - if (wp->broken == NIL) { - lispobj *header = (lispobj *) PTR(wp->value); - DPRINTF(debug_static_array_p, - (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj*) wp->value, *header)); + DPRINTF(debug_static_array_p, + (stdout, " wp %p value %p header 0x%08lx\n", + wp, (lispobj*) wp->value, *header)); - /* - * Only free the arrays where the mark bit is clear. - */ - if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { - lispobj *static_array = (lispobj *) PTR(wp->value); - DPRINTF(debug_static_array_p, - (stdout, " Free static vector\n")); + gc_assert(wp->broken == NIL); + /* + * Only free the arrays where the mark bit is clear. + */ + if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { + lispobj *static_array = (lispobj *) PTR(wp->value); + DPRINTF(debug_static_array_p, + (stdout, " Free static vector\n")); - wp->value = NIL; - wp->broken = T; + wp->value = NIL; + wp->broken = T; - free(static_array); - } + free(static_array); } + wp = next; } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/a9b2d0819cfdeb20badfe5a… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/a9b2d0819cfdeb20badfe5a… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] 2 commits: Remove broken weak pointers from the list
by Raymond Toy (＠rtoy) 21 Feb '24

21 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: a86a44f8 by Raymond Toy at 2024-02-21T13:09:45-08:00 Remove broken weak pointers from the list In phase 2 where we break weak pointers that point to a static vector that we've already visited, we can delete this broken weak pointer from our list. We don't need to do anything more with this weak pointer. Likewise in phase 3, when we break the weak pointer to unused static vector, we can also remove this weak pointer from the list since there's nothing more we need to do. In phase 3 and phase 4, add an assert that the vector we're examining is not broken. They should have been removed in the previous phase. - - - - - 24d8420b by Raymond Toy at 2024-02-21T13:45:47-08:00 Revert "Remove broken weak pointers from the list" This reverts commit a86a44f8f6d9fbe69a694974c9fe4734e81e48cb. I think this is mostly right, but I think it's not handling the case if the first element is removed or the case where the last element is removed. I need to think about this more to cover the corner cases. - - - - - 0 changed files: Changes: View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/c636346ff569ab2a1ea9f3… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/c636346ff569ab2a1ea9f3… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Actually need pointer to header
by Raymond Toy (＠rtoy) 20 Feb '24

20 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: c636346f by Raymond Toy at 2024-02-19T16:05:08-08:00 Actually need pointer to header In a previous commit, we changed `*header` to `header`. But we need to be able to modify the header word to set the visited bit. Hence revert the change and make `header` a pointer to the header word. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5433,20 +5433,20 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * visited the static vector, break the weak pointer. */ for (wp = static_vector_list; wp; wp = wp->next) { - lispobj header = *(lispobj *) PTR(wp->value); + lispobj *header = (lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj *) wp->value, header)); + wp, (lispobj *) wp->value, *header)); /* * If the static vector is unused (mark bit clear) and if we * haven't seen this vector before, set the visited flag. If * we have visited this vector before, break the weak pointer. */ - if ((header & STATIC_VECTOR_MARK_BIT) == 0) { + if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { /* Unused static vector */ - if ((header & STATIC_VECTOR_VISITED_BIT) == 0) { + if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) { DPRINTF(debug_static_array_p, (stdout, " Mark vector\n")); *header |= STATIC_VECTOR_VISITED_BIT; View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c636346ff569ab2a1ea9f36… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c636346ff569ab2a1ea9f36… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Apply 2 suggestion(s) to 1 file(s)
by Raymond Toy (＠rtoy) 20 Feb '24

20 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: e54fa13e by Carl Shapiro at 2024-02-19T23:59:56+00:00 Apply 2 suggestion(s) to 1 file(s) - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5423,48 +5423,6 @@ scan_static_vectors(struct weak_pointer *static_vector_list) { struct weak_pointer *wp; -#if 0 - if (debug_static_array_p) { - printf("Phase 1: Find static vectors\n"); - } - - /* - * Find weak pointers to static arrays, using a linked list. We - * reuse the next slot of the weak pointer to chain these weak - * pointers together. - * - * Invariant: static_vector_list only has weak pointers to static - * vectors. - */ - wp = weak_pointers; - while (wp) { - lispobj value = wp->value; - struct weak_pointer *next = wp->next; - - if (Pointerp(value)) { - /* The value may be a static vector */ - lispobj header = *(lispobj *) PTR(value); - - if (maybe_static_array_p(header)) { - - if (debug_static_array_p) { - printf(" Add: wp %p value %p header 0x%08lx, next = %p\n", - wp, (lispobj *) wp->value, header, wp->next); - } - - wp->next = static_vector_list; - static_vector_list = wp; - } else { - if (debug_static_array_p) { - printf(" Skip: wp %p value %p header 0x%08lx\n", - wp, (lispobj *) wp->value, header); - } - } - } - wp = next; - } -#endif - DPRINTF(debug_static_array_p, (stdout, "Phase 2: Visit unused static vectors\n")); @@ -5475,20 +5433,20 @@ scan_static_vectors(struct weak_pointer *static_vector_list) * visited the static vector, break the weak pointer. */ for (wp = static_vector_list; wp; wp = wp->next) { - lispobj *header = (lispobj *) PTR(wp->value); + lispobj header = *(lispobj *) PTR(wp->value); DPRINTF(debug_static_array_p, (stdout, " wp %p value %p header 0x%08lx\n", - wp, (lispobj *) wp->value, *header)); + wp, (lispobj *) wp->value, header)); /* * If the static vector is unused (mark bit clear) and if we * haven't seen this vector before, set the visited flag. If * we have visited this vector before, break the weak pointer. */ - if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { + if ((header & STATIC_VECTOR_MARK_BIT) == 0) { /* Unused static vector */ - if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) { + if ((header & STATIC_VECTOR_VISITED_BIT) == 0) { DPRINTF(debug_static_array_p, (stdout, " Mark vector\n")); *header |= STATIC_VECTOR_VISITED_BIT; View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/e54fa13e3c3eb74d8107eed… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/e54fa13e3c3eb74d8107eed… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0