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

[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

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Add comment about where STATIC_VECTOR_HEADER_BIT is
by Raymond Toy (＠rtoy) 19 Feb '24

19 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 1b0121ea by Raymond Toy at 2024-02-19T09:17:53-08:00 Add comment about where STATIC_VECTOR_HEADER_BIT is I sometimes forget that `STATIC_VECTOR_HEADER_BIT` is relative to the header value, not the actual header. Make it a bit clearer where this bit is. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -34,7 +34,8 @@ /* * If the header value for a vector has this bit set, then it is a - * static vector. + * static vector. NOTE: This is a bit in the header value of a + * header, NOT the bit in the full header! */ #define STATIC_VECTOR_HEADER_BIT 0x1 View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/1b0121ea2b6f3ac906ca80d… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/1b0121ea2b6f3ac906ca80d… 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 phase 1 of scan_static_vectors to scan_weak_pointers
by Raymond Toy (＠rtoy) 19 Feb '24

19 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: c4d390b4 by Raymond Toy at 2024-02-19T09:10:10-08:00 Move phase 1 of scan_static_vectors to scan_weak_pointers As suggested in the review, moved the search for static vectors from `scan_static_vectors` to `scan_weak_pointers`. This reduces the number of traversals of the weak pointers to just the one. Also modified the debugging prints to use `DPRINTF` instead. Slightly less cluttered looking code. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5418,11 +5418,11 @@ size_weak_pointer(lispobj * where) static void -scan_static_vectors(void) +scan_static_vectors(struct weak_pointer *static_vector_list) { struct weak_pointer *wp; - struct weak_pointer *static_vector_list = NULL; +#if 0 if (debug_static_array_p) { printf("Phase 1: Find static vectors\n"); } @@ -5462,10 +5462,10 @@ scan_static_vectors(void) } wp = next; } +#endif - if (debug_static_array_p) { - printf("Phase 2: Visit unused static vectors\n"); - } + DPRINTF(debug_static_array_p, + (stdout, "Phase 2: Visit unused static vectors\n")); /* * static_vector_list now points to all weak pointers to static @@ -5476,10 +5476,9 @@ scan_static_vectors(void) for (wp = static_vector_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); - if (debug_static_array_p) { - printf(" 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)); /* * If the static vector is unused (mark bit clear) and if we @@ -5489,15 +5488,12 @@ scan_static_vectors(void) if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { /* Unused static vector */ if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) { - if (debug_static_array_p) { - printf(" Mark vector\n"); - } + DPRINTF(debug_static_array_p, (stdout, " Mark vector\n")); *header |= STATIC_VECTOR_VISITED_BIT; } else { - if (debug_static_array_p) { - printf(" Break weak pointer %p\n", wp); - } + DPRINTF(debug_static_array_p, + (stdout, " Break weak pointer %p\n", wp)); wp->value = NIL; wp->broken = T; @@ -5506,9 +5502,8 @@ scan_static_vectors(void) } - if (debug_static_array_p) { - printf("Phase 3: Free static vectors\n"); - } + DPRINTF(debug_static_array_p, + (stdout, "Phase 3: Free static vectors\n")); /* * static_vector_list now contains either broken weak pointers or @@ -5524,19 +5519,17 @@ scan_static_vectors(void) if (wp->broken == NIL) { lispobj *header = (lispobj *) PTR(wp->value); - if (debug_static_array_p) { - printf(" 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); - if (debug_static_array_p) { - printf(" Free static vector\n"); - } + DPRINTF(debug_static_array_p, + (stdout, " Free static vector\n")); wp->value = NIL; wp->broken = T; @@ -5547,9 +5540,8 @@ scan_static_vectors(void) } - if (debug_static_array_p) { - printf("Phase 4: unmark static vectors\n"); - } + DPRINTF(debug_static_array_p, + (stdout, "Phase 4: unmark static vectors\n")); /* * At this point, static_vector_list contains weak pointers that @@ -5563,15 +5555,13 @@ scan_static_vectors(void) if (wp->broken == NIL) { lispobj *header = (lispobj *) PTR(wp->value); - if (debug_static_array_p) { - printf(" wp %p value %p broken %d header 0x%08lx\n", - wp, (lispobj*) wp->value, wp->broken == T, *header); - } + 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) { - if (debug_static_array_p) { - printf(" Clearing mark bit\n"); - } + DPRINTF(debug_static_array_p, + (stdout, " Clearing mark bit\n")); *header &= ~STATIC_VECTOR_MARK_BIT; } @@ -5582,24 +5572,57 @@ scan_static_vectors(void) void scan_weak_pointers(void) { - struct weak_pointer *wp; + struct weak_pointer *wp = weak_pointers; + struct weak_pointer *static_vector_list = NULL; - for (wp = weak_pointers; wp; wp = wp->next) { + /* + * Scan through all the weak pointers, updating the value or + * breaking the weak pointer. + * + * 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 static vectors together. + */ + DPRINTF(debug_static_array_p, + (stdout, "Phase 0: Process weak pointers\n")); + + 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) && from_space_p(value)) { - if (first_pointer[0] == 0x01) - wp->value = first_pointer[1]; - else { - wp->value = NIL; - wp->broken = T; - } - } + if (Pointerp(value)) { + if (from_space_p(value)) { + if (first_pointer[0] == 0x01) + wp->value = first_pointer[1]; + else { + wp->value = NIL; + wp->broken = T; + } + } else { + /* The value may be a static vector */ + 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)); + + wp->next = static_vector_list; + static_vector_list = wp; + } else { + DPRINTF(debug_static_array_p, + (stdout, " Skip: wp %p value %p header 0x%08lx\n", + wp, (lispobj *) wp->value, header)); + } + } + } + wp = next; } - scan_static_vectors(); + scan_static_vectors(static_vector_list); } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c4d390b43ec0ee791e16e16… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c4d390b43ec0ee791e16e16… 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] 5 commits: Put back tabs to match master branch
by Raymond Toy (＠rtoy) 18 Feb '24

18 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 9c0971b2 by Raymond Toy at 2024-02-18T14:01:01-08:00 Put back tabs to match master branch This minimizes the difference between this and the master. - - - - - 025f7c08 by Raymond Toy at 2024-02-18T14:04:32-08:00 Add comment for phase 4 of scanning static vectors. - - - - - b5653912 by Raymond Toy at 2024-02-18T14:18:55-08:00 Be more careful about clearing the static vector mark bit Check that the weak pointer hasn't been broken yet before trying to clear the mark bit of the static vector. I think this currently doesn't matter because when the weak pointer is broken, the value is set to NIL, and NIL doesn't have the MSB set. But that's not guaranteed, so let's be safe. - - - - - 2478e670 by Raymond Toy at 2024-02-18T14:26:02-08:00 For phase 3 and 4, onl process weak pointers that are not broken For a little extra safety, only process weak pointers that have not been broken, so add the check at the very beginning of the loop. - - - - - 84c364a2 by Raymond Toy at 2024-02-18T14:32:20-08:00 Add some more comments - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5442,13 +5442,13 @@ scan_static_vectors(void) if (Pointerp(value)) { /* The value may be a static vector */ - lispobj *header = (lispobj *) PTR(value); + lispobj header = *(lispobj *) PTR(value); - if (maybe_static_array_p(*header)) { + 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, (lispobj *) wp->value, header, wp->next); } wp->next = static_vector_list; @@ -5456,7 +5456,7 @@ scan_static_vectors(void) } else { if (debug_static_array_p) { printf(" Skip: wp %p value %p header 0x%08lx\n", - wp, (lispobj *) wp->value, *header); + wp, (lispobj *) wp->value, header); } } } @@ -5472,7 +5472,6 @@ scan_static_vectors(void) * 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. - * */ for (wp = static_vector_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); @@ -5512,23 +5511,27 @@ scan_static_vectors(void) } /* + * 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. */ for (wp = static_vector_list; wp; wp = wp->next) { - lispobj *header = (lispobj *) PTR(wp->value); + /* Skip over broken weak pointers */ + if (wp->broken == NIL) { + lispobj *header = (lispobj *) PTR(wp->value); - if (debug_static_array_p) { - printf(" wp %p value %p header 0x%08lx\n", - wp, (lispobj*) wp->value, *header); - } + if (debug_static_array_p) { + printf(" wp %p value %p header 0x%08lx\n", + wp, (lispobj*) wp->value, *header); + } - /* - * Only free the arrays where the mark bit is clear. - */ - if (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); if (debug_static_array_p) { @@ -5542,25 +5545,36 @@ scan_static_vectors(void) } } } + if (debug_static_array_p) { printf("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 . + */ for (wp = static_vector_list; wp; wp = wp->next) { - lispobj *header = (lispobj *) PTR(wp->value); - - if (debug_static_array_p) { - printf(" wp %p value %p broken %d header 0x%08lx\n", - wp, (lispobj*) wp->value, wp->broken == T, *header); - } + /* Skip over broken weak pointers */ + if (wp->broken == NIL) { + lispobj *header = (lispobj *) PTR(wp->value); - if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { if (debug_static_array_p) { - printf(" Clearing mark bit\n"); + printf(" wp %p value %p broken %d header 0x%08lx\n", + wp, (lispobj*) wp->value, wp->broken == T, *header); } - *header &= ~STATIC_VECTOR_MARK_BIT; + if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { + if (debug_static_array_p) { + printf(" Clearing mark bit\n"); + } + + *header &= ~STATIC_VECTOR_MARK_BIT; + } } } } @@ -5576,13 +5590,13 @@ scan_weak_pointers(void) wp->mark_bit = NIL; if (Pointerp(value) && from_space_p(value)) { - if (first_pointer[0] == 0x01) - wp->value = first_pointer[1]; - else { - wp->value = NIL; - wp->broken = T; - } - } + if (first_pointer[0] == 0x01) + wp->value = first_pointer[1]; + else { + wp->value = NIL; + wp->broken = T; + } + } } scan_static_vectors(); View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/9cb44e305299b41e951c58… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/9cb44e305299b41e951c58… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0