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

[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

[Git][cmucl/cmucl][issue-243-weak-pointer-to-static-array] Fix typo bug and remove comment
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: 9cb44e30 by Raymond Toy at 2024-02-17T19:59:54-08:00 Fix typo bug and remove comment In merging the if statements we should have used `&&` instead of `&`. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5570,16 +5570,12 @@ scan_weak_pointers(void) { struct weak_pointer *wp; - /* - * Now process the weak pointers. - */ - for (wp = weak_pointers; wp; wp = wp->next) { lispobj value = wp->value; lispobj *first_pointer = (lispobj *) PTR(value); wp->mark_bit = NIL; - if (Pointerp(value) & from_space_p(value)) { + if (Pointerp(value) && from_space_p(value)) { if (first_pointer[0] == 0x01) wp->value = first_pointer[1]; else { View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/9cb44e305299b41e951c584… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/9cb44e305299b41e951c584… 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] Minor tweak to minimize diff from master
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: f45625b4 by Raymond Toy at 2024-02-17T19:54:26-08:00 Minor tweak to minimize diff from master Combine two if statements like it was on master to minimize the diff between this version and master because it's a needless change. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5579,14 +5579,12 @@ scan_weak_pointers(void) lispobj *first_pointer = (lispobj *) PTR(value); wp->mark_bit = NIL; - 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; - } + if (Pointerp(value) & from_space_p(value)) { + if (first_pointer[0] == 0x01) + wp->value = first_pointer[1]; + else { + wp->value = NIL; + wp->broken = T; } } } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/f45625b4992eada575b56dd… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/f45625b4992eada575b56dd… 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 clear-static-vector-mark
by Raymond Toy (＠rtoy) 17 Feb '24

17 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 4ccd8d2a by Raymond Toy at 2024-02-17T12:58:07-08:00 Remove unneeded clear-static-vector-mark Remove `clear-static-vector-mark` because GC does that for us now in `scan_static_vectors`. Fix up some comments in `finalize-static-vectors`, and be a bit more careful in only removing an item from `lisp::*static-vectors*` only if the weak pointer was broken. Minor tweak to a debugging print. Update cmucl.pot. - - - - - 3 changed files: - src/code/array.lisp - src/i18n/locale/cmucl.pot - src/lisp/gencgc.c Changes: ===================================== src/code/array.lisp ===================================== @@ -374,40 +374,20 @@ sys:system-area-pointer)) (sys:int-sap addr))))) -(defun clear-static-vector-mark () - ;; Run down the list of weak pointers to static vectors. For each - ;; vector, clear the mark. - (dolist (wp *static-vectors*) - (let ((vector (weak-pointer-value wp))) - ;; The value should never be NIL here? - (when vector - (let* ((sap (sys:vector-sap vector)) - (header (sys:sap-ref-32 sap (* -2 vm:word-bytes)))) - (when *debug-static-array-p* - (format t (intl:gettext "static vector ~A. header = ~X~%") - vector header)) - (setf (sys:sap-ref-32 sap (* -2 vm:word-bytes)) - (logand header #x7fffffff))))))) - (defun finalize-static-vectors () - ;; Run down the list of weak-pointers to static vectors. Look at - ;; the static vector and see if vector is marked. If so, clear the - ;; mark, and do nothing. If the mark is not set, then the vector is - ;; free, so free it, and remove this weak-pointer from the list. - ;; The mark bit the MSB of the header word. Look at scavenge in - ;; gencgc.c. + ;; Run down the list of weak-pointers to static vectors and remove + ;; any weak pointers that have been broken. (when *static-vectors* (when *debug-static-array-p* (let ((*print-array* nil)) (format t (intl:gettext "Finalizing static vectors ~S~%") *static-vectors*))) - ;; Remove any weak pointers that whose value is NIL. The + ;; Remove any weak pointers that have been broken. The ;; corresponding static array has been freed by GC. (setf *static-vectors* - (delete-if-not #'weak-pointer-value *static-vectors*)))) + (delete-if-not #'(lambda (wp) + (nth-value 1 (weak-pointer-value wp))) + *static-vectors*)))) -;; Clear the mark bit of all of static vectors before GC -#+nil -(pushnew 'clear-static-vector-mark *before-gc-hooks*) ;; Clean up any unreferenced static vectors after GC has run. (pushnew 'finalize-static-vectors *after-gc-hooks*) ===================================== src/i18n/locale/cmucl.pot ===================================== @@ -2379,10 +2379,6 @@ msgstr "" msgid "~&Freeing foreign vector at #x~X~%" msgstr "" -#: src/code/array.lisp -msgid "static vector ~A. header = ~X~%" -msgstr "" - #: src/code/array.lisp msgid "Finalizing static vectors ~S~%" msgstr "" ===================================== src/lisp/gencgc.c ===================================== @@ -5532,7 +5532,7 @@ scan_static_vectors(void) if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { lispobj *static_array = (lispobj *) PTR(wp->value); if (debug_static_array_p) { - printf(" Free wp\n"); + printf(" Free static vector\n"); } wp->value = NIL; View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/4ccd8d2a9bbd8d4965722c7… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/4ccd8d2a9bbd8d4965722c7… 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] Turn off debug_static_array_p
by Raymond Toy (＠rtoy) 17 Feb '24

17 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 18bdcf0a by Raymond Toy at 2024-02-17T09:32:53-08:00 Turn off debug_static_array_p - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -260,7 +260,7 @@ unsigned counters_verbose = 0; * If true, then some debugging information is printed when scavenging * static (malloc'ed) arrays. */ -boolean debug_static_array_p = 1; +boolean debug_static_array_p = 0; /* * To enable the use of page protection to help avoid the scavenging View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/18bdcf0a5f76cf17d6005b3… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/18bdcf0a5f76cf17d6005b3… 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] 3 commits: Don't add to the *before-gc-hooks* to clear static vector marks.
by Raymond Toy (＠rtoy) 17 Feb '24

17 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 5472c92c by Raymond Toy at 2024-02-16T16:05:27-08:00 Don't add to the *before-gc-hooks* to clear static vector marks. - - - - - 060bbae4 by Raymond Toy at 2024-02-17T08:43:19-08:00 Clean up implementation Add `#defines` for the various bits needed to handle processing of static arrays. When scavenging weak pointers to static arrays, clear out the visited bit, just to be safe. Clean up debugging prints. Add phase 4 to go through all the remaining static vectors and unmark them so we don't have to do that in Lisp. - - - - - 255af7a5 by Raymond Toy at 2024-02-17T09:03:01-08:00 Clean up prints some more - - - - - 2 changed files: - src/code/array.lisp - src/lisp/gencgc.c Changes: ===================================== src/code/array.lisp ===================================== @@ -406,6 +406,7 @@ (delete-if-not #'weak-pointer-value *static-vectors*)))) ;; Clear the mark bit of all of static vectors before GC +#+nil (pushnew 'clear-static-vector-mark *before-gc-hooks*) ;; Clean up any unreferenced static vectors after GC has run. (pushnew 'finalize-static-vectors *after-gc-hooks*) ===================================== src/lisp/gencgc.c ===================================== @@ -32,6 +32,25 @@ */ #define EQ_BASED_HASH_VALUE 0x80000000 +/* + * If the header value for a vector has this bit set, then it is a + * static vector. + */ +#define STATIC_VECTOR_HEADER_BIT 0x1 + +/* + * Mark bit for static vectors. If set in the header, the static + * vector is in use. + */ +#define STATIC_VECTOR_MARK_BIT 0x80000000 + +/* + * Visited bit for static vectors. When scanning weak pointers for + * static vectors, this bit indicates that we've visited this static + * vector already. + */ +#define STATIC_VECTOR_VISITED_BIT 0x08000000 + #define gc_abort() lose("GC invariant lost! File \"%s\", line %d\n", \ __FILE__, __LINE__) @@ -241,7 +260,7 @@ unsigned counters_verbose = 0; * If true, then some debugging information is printed when scavenging * static (malloc'ed) arrays. */ -boolean debug_static_array_p = 0; +boolean debug_static_array_p = 1; /* * To enable the use of page protection to help avoid the scavenging @@ -2718,13 +2737,14 @@ scav_static_vector(lispobj object) ptr, (unsigned long) header); } - static_p = (HeaderValue(header) & 1) == 1; + static_p = (HeaderValue(header) & STATIC_VECTOR_HEADER_BIT) == 1; if (static_p) { /* - * We have a static vector. Mark it as - * reachable by setting the MSB of the header. + * We have a static vector. Mark it as reachable by + * setting the MSB of the header. And clear out any + * possible visited bit. */ - *ptr = header | 0x80000000; + *ptr = (header | STATIC_VECTOR_MARK_BIT) & ~STATIC_VECTOR_VISITED_BIT; if (debug_static_array_p) { fprintf(stderr, "Scavenged static vector @%p, header = 0x%lx\n", ptr, (unsigned long) header); @@ -5402,10 +5422,9 @@ scan_static_vectors(void) { struct weak_pointer *wp; struct weak_pointer *static_vector_list = NULL; - const int scan_mark_flag = 0x8000; if (debug_static_array_p) { - printf("Phase 1: find static vectors\n"); + printf("Phase 1: Find static vectors\n"); } /* @@ -5428,15 +5447,16 @@ scan_static_vectors(void) if (maybe_static_array_p(*header)) { if (debug_static_array_p) { - printf("Adding %p header = 0x%08lx, next = %p\n", - wp, *header, wp->next); + 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("Skipping %p header = 0x%08lx\n", wp, *header); + printf(" Skip: wp %p value %p header 0x%08lx\n", + wp, (lispobj *) wp->value, *header); } } } @@ -5444,7 +5464,7 @@ scan_static_vectors(void) } if (debug_static_array_p) { - printf("Phase 2\n"); + printf("Phase 2: Visit unused static vectors\n"); } /* @@ -5458,25 +5478,26 @@ scan_static_vectors(void) lispobj *header = (lispobj *) PTR(wp->value); if (debug_static_array_p) { - printf("wp %p value 0x%08lx header 0x%08lx\n", - wp, wp->value, *header); + printf(" 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 - * haven't seen this vector before, set the scan flag. + * haven't seen this vector before, set the visited flag. If + * we have visited this vector before, break the weak pointer. */ - if ((*header & 0x80000000) == 0) { + if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { /* Unused static vector */ - if ((*header & scan_mark_flag) == 0) { + if ((*header & STATIC_VECTOR_VISITED_BIT) == 0) { if (debug_static_array_p) { - printf(" Mark vector\n"); + printf(" Mark vector\n"); } - *header |= scan_mark_flag; + *header |= STATIC_VECTOR_VISITED_BIT; } else { if (debug_static_array_p) { - printf(" Break weak pointer %p\n", wp); + printf(" Break weak pointer %p\n", wp); } wp->value = NIL; @@ -5499,16 +5520,19 @@ scan_static_vectors(void) for (wp = static_vector_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); - printf("wp = %p, header = 0x%08lx\n", wp, *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) { - if ((*header & 0x80000000) == 0) { + if ((*header & STATIC_VECTOR_MARK_BIT) == 0) { lispobj *static_array = (lispobj *) PTR(wp->value); if (debug_static_array_p) { - printf("free wp %p: %p\n", wp, static_array); + printf(" Free wp\n"); } wp->value = NIL; @@ -5518,6 +5542,27 @@ scan_static_vectors(void) } } } + + if (debug_static_array_p) { + printf("Phase 4: unmark static vectors\n"); + } + + 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); + } + + if ((*header & STATIC_VECTOR_MARK_BIT) != 0) { + if (debug_static_array_p) { + printf(" Clearing mark bit\n"); + } + + *header &= ~STATIC_VECTOR_MARK_BIT; + } + } } void View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/959be526befaead7b46d43… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/compare/959be526befaead7b46d43… 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] Create list of all static vectors, not just the unmarked ones
by Raymond Toy (＠rtoy) 16 Feb '24

16 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 959be526 by Raymond Toy at 2024-02-16T13:08:57-08:00 Create list of all static vectors, not just the unmarked ones Instead of creating a list of unmarked static vectors, create a list of all static vectors. Then we can process the unmarked vectors and free them carefully. Although we haven't implemented it yet, this allows us to make one final pass to unmark the marked static vectors so that when GC happens again, we can mark them as being in use. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5401,19 +5401,20 @@ static void scan_static_vectors(void) { struct weak_pointer *wp; - struct weak_pointer *clearable_list = NULL; + struct weak_pointer *static_vector_list = NULL; + const int scan_mark_flag = 0x8000; if (debug_static_array_p) { - printf("Phase 1: build clearable list\n"); + printf("Phase 1: find static vectors\n"); } /* - * Find weak pointers to unmarked static arrays, using a linked - * list. We reuse the next slot ofthe weak pointer to chain these - * weak pointers together. + * 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: clearable_list only has weak pointers to unmarked - * static vectors. + * Invariant: static_vector_list only has weak pointers to static + * vectors. */ wp = weak_pointers; while (wp) { @@ -5424,16 +5425,15 @@ scan_static_vectors(void) /* The value may be a static vector */ lispobj *header = (lispobj *) PTR(value); - if (maybe_static_array_p(*header) - && (HeaderValue(*header) == 1)) { + if (maybe_static_array_p(*header)) { if (debug_static_array_p) { printf("Adding %p header = 0x%08lx, next = %p\n", wp, *header, wp->next); } - wp->next = clearable_list; - clearable_list = wp; + wp->next = static_vector_list; + static_vector_list = wp; } else { if (debug_static_array_p) { printf("Skipping %p header = 0x%08lx\n", wp, *header); @@ -5448,14 +5448,13 @@ scan_static_vectors(void) } /* - * clearable_list now points to all weak pointers to unmarked - * static vectors. Go through the list. If it's not marked, mark - * it. If it's marked, break the weak pointer. + * 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. * - * Invariant: clearable_list contains only weak pointers that have - * been broken or that point to a unique dead static vector. */ - for (wp = clearable_list; wp; wp = wp->next) { + for (wp = static_vector_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); if (debug_static_array_p) { @@ -5463,42 +5462,60 @@ scan_static_vectors(void) wp, wp->value, *header); } - if (HeaderValue(*header) == 1) { - if (debug_static_array_p) { - printf(" Mark vector\n"); - } + /* + * If the static vector is unused (mark bit clear) and if we + * haven't seen this vector before, set the scan flag. + */ + if ((*header & 0x80000000) == 0) { + /* Unused static vector */ + if ((*header & scan_mark_flag) == 0) { + if (debug_static_array_p) { + printf(" Mark vector\n"); + } - *header |= 0x80000000; - } else { - if (debug_static_array_p) { - printf(" Break weak pointer %p\n", wp); - } + *header |= scan_mark_flag; + } else { + if (debug_static_array_p) { + printf(" Break weak pointer %p\n", wp); + } - wp->value = NIL; - wp->broken = T; + wp->value = NIL; + wp->broken = T; + } } } + if (debug_static_array_p) { printf("Phase 3: Free static vectors\n"); } /* - * Free up space. Go through clearable_list and for each weak - * pointer that has not been broken, we can free the space. Then - * break the weak pointer too, since the space has been freed. + * 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 = clearable_list; wp; wp = wp->next) { + for (wp = static_vector_list; wp; wp = wp->next) { + lispobj *header = (lispobj *) PTR(wp->value); + + printf("wp = %p, header = 0x%08lx\n", wp, *header); + + /* + * Only free the arrays where the mark bit is clear. + */ if (wp->broken == NIL) { - lispobj *static_array = (lispobj *) PTR(wp->value); - if (debug_static_array_p) { - printf("free wp %p: %p\n", wp, static_array); - } + if ((*header & 0x80000000) == 0) { + lispobj *static_array = (lispobj *) PTR(wp->value); + if (debug_static_array_p) { + printf("free wp %p: %p\n", wp, static_array); + } - wp->value = NIL; - wp->broken = T; + wp->value = NIL; + wp->broken = T; - free(static_array); + free(static_array); + } } } } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/959be526befaead7b46d43b… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/959be526befaead7b46d43b… 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] Clean up prints
by Raymond Toy (＠rtoy) 16 Feb '24

16 Feb '24

Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: c587dab3 by Raymond Toy at 2024-02-16T10:30:56-08:00 Clean up prints Only print debugging stuff if `debug_static_array_p` is true. Remove prints from `scan_weak_pointers`. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -5403,7 +5403,9 @@ scan_static_vectors(void) struct weak_pointer *wp; struct weak_pointer *clearable_list = NULL; - printf("Phase 1: build clearable list\n"); + if (debug_static_array_p) { + printf("Phase 1: build clearable list\n"); + } /* * Find weak pointers to unmarked static arrays, using a linked @@ -5424,18 +5426,26 @@ scan_static_vectors(void) if (maybe_static_array_p(*header) && (HeaderValue(*header) == 1)) { - printf("Adding %p header = 0x%08lx, next = %p\n", - wp, *header, wp->next); + + if (debug_static_array_p) { + printf("Adding %p header = 0x%08lx, next = %p\n", + wp, *header, wp->next); + } + wp->next = clearable_list; clearable_list = wp; } else { - printf("Skipping %p header = 0x%08lx\n", wp, *header); + if (debug_static_array_p) { + printf("Skipping %p header = 0x%08lx\n", wp, *header); + } } } wp = next; } - printf("Phase 2\n"); + if (debug_static_array_p) { + printf("Phase 2\n"); + } /* * clearable_list now points to all weak pointers to unmarked @@ -5448,19 +5458,30 @@ scan_static_vectors(void) for (wp = clearable_list; wp; wp = wp->next) { lispobj *header = (lispobj *) PTR(wp->value); - printf("wp %p value 0x%08lx header 0x%08lx\n", - wp, wp->value, *header); + if (debug_static_array_p) { + printf("wp %p value 0x%08lx header 0x%08lx\n", + wp, wp->value, *header); + } + if (HeaderValue(*header) == 1) { - printf(" Mark vector\n"); + if (debug_static_array_p) { + printf(" Mark vector\n"); + } + *header |= 0x80000000; } else { - printf(" Break weak pointer %p\n", wp); + if (debug_static_array_p) { + printf(" Break weak pointer %p\n", wp); + } + wp->value = NIL; wp->broken = T; } } - printf("Phase 3: Free static vectors\n"); + if (debug_static_array_p) { + printf("Phase 3: Free static vectors\n"); + } /* * Free up space. Go through clearable_list and for each weak @@ -5470,7 +5491,9 @@ scan_static_vectors(void) for (wp = clearable_list; wp; wp = wp->next) { if (wp->broken == NIL) { lispobj *static_array = (lispobj *) PTR(wp->value); - printf("free wp %p: %p\n", wp, static_array); + if (debug_static_array_p) { + printf("free wp %p: %p\n", wp, static_array); + } wp->value = NIL; wp->broken = T; @@ -5489,15 +5512,10 @@ scan_weak_pointers(void) * Now process the weak pointers. */ - printf("scan_weak_pointers...\n"); - for (wp = weak_pointers; wp; wp = wp->next) { lispobj value = wp->value; lispobj *first_pointer = (lispobj *) PTR(value); - printf("scan_weak: wp = %p value %p header 0x%08lx\n", - wp, (lispobj*) value, *first_pointer); - wp->mark_bit = NIL; if (Pointerp(value)) { if (from_space_p(value)) { @@ -5510,7 +5528,6 @@ scan_weak_pointers(void) } } } - printf("scan_weak_pointers done\n"); scan_static_vectors(); } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c587dab3901ca26fbc6494b… -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/c587dab3901ca26fbc6494b… You're receiving this email because of your account on gitlab.common-lisp.net.

1 0