Subject: feature/igc: [PATCH] Make charset_table an exact root

From: Helmut Eller <[email-masked]>
Date: Tue, 03 Jun 2025 21:43:52 +0200

Here is a proposal to move charset_table out of the GC heap and make it
an exact root area instead.

>From ded35d23523a18b5c61dd7e7c2de92cb2dafc7ff Mon Sep 17 00:00:00 2001
From: Helmut Eller <[email-masked]>
Date: Tue, 3 Jun 2025 17:30:33 +0200
Subject: [PATCH] Make charset_table an exact root

Previously, the charset_table was located in various places:

1. in charset_table_init during bootstrap (ambiguous root)
2. on the GC heap (loaded from the dump; pinned)
3. on the malloc heap when it needed to grow (ambiguous root)

Now, the charset table is copied back to charset_table_init after
loading the dump and it is scanned as an exact root area.

* src/igc.h (igc_root_create_area): New API.
(igc_scan_charset_table): New.
* src/igc.c (igc_scan_charset_table): Implementation.
(root_create_charset_table): Use it.
(clone_charset_table, clone_charset): New helpers.
(igc_on_pdump_loaded): Use it.
(fix_charset_table): Deleted.
* src/charset.c (Fdefine_charset_internal): Create/destory root areas
when allocating/freeing charset tables.
---
 src/charset.c | 19 +++++++-----
 src/igc.c     | 84 ++++++++++++++++++++++++++++++---------------------
 src/igc.h     |  4 +++
 3 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/src/charset.c b/src/charset.c
index 9cbc899b354..2ee6fa0345c 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -1132,17 +1132,20 @@ DEFUN ("define-charset-internal", Fdefine_charset_internal,
 	     coding_system.charbuf[i] entries, which are 'int'.  */
 	  int old_size = charset_table_size;
 	  ptrdiff_t new_size = old_size;
-	  struct charset *new_table =
+	  struct charset *new_table
+	    = xpalloc (0, &new_size, 1,
+		       min (INT_MAX, MOST_POSITIVE_FIXNUM),
+		       sizeof *charset_table);
 #ifdef HAVE_MPS
-	    igc_xpalloc_ambig
-#else
-	    xpalloc
+	  memset (new_table, 0, new_size * sizeof *new_table);
+	  igc_root_create_area (new_table, new_table + new_size,
+			        igc_scan_charset_table, NULL, "charset-table");
 #endif
-	    (0, &new_size, 1,
-	     min (INT_MAX, MOST_POSITIVE_FIXNUM),
-	     sizeof *charset_table);
           memcpy (new_table, charset_table, old_size * sizeof *new_table);
-          charset_table = new_table;
+#ifdef HAVE_MPS
+	  igc_destroy_root_with_start (charset_table);
+#endif
+	  charset_table = new_table;
 	  charset_table_size = new_size;
 	  /* FIXME: This leaks memory, as the old charset_table becomes
 	     unreachable.  If the old charset table is charset_table_init
diff --git a/src/igc.c b/src/igc.c
index f63b0fe8c88..67aefb1b273 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -1725,6 +1725,20 @@ scan_tty_list (mps_ss_t ss, void *start, void *end, void *closure)
   return MPS_RES_OK;
 }
 
+mps_res_t
+igc_scan_charset_table (struct igc_ss *s, void *start, void *end, void *closure)
+{
+  mps_ss_t ss = (mps_ss_t) s;
+  struct charset *table = start;
+  MPS_SCAN_BEGIN (ss)
+  {
+    for (size_t i = 0, used = charset_table_used; i < used; i++)
+      IGC_FIX12_OBJ (ss, &table[i].attributes);
+  }
+  MPS_SCAN_END (ss);
+  return MPS_RES_OK;
+}
+
 /***********************************************************************
 			 Default pad, fwd, ...
  ***********************************************************************/
@@ -1941,20 +1955,6 @@ fix_handler (mps_ss_t ss, struct handler *h)
   return MPS_RES_OK;
 }
 
-static mps_res_t
-fix_charset_table (mps_ss_t ss, struct charset *table, size_t nbytes)
-{
-  igc_assert (table == charset_table);
-  igc_assert (nbytes == (charset_table_size * sizeof (struct charset)));
-  MPS_SCAN_BEGIN (ss)
-  {
-    for (size_t i = 0, len = nbytes / sizeof (struct charset); i < len; i++)
-      IGC_FIX12_OBJ (ss, &table[i].attributes);
-  }
-  MPS_SCAN_END (ss);
-  return MPS_RES_OK;
-}
-
 static mps_res_t fix_vector (mps_ss_t ss, struct Lisp_Vector *v);
 static mps_res_t fix_marker_vector (mps_ss_t ss, struct Lisp_Vector *v);
 static mps_res_t fix_weak_hash_table_strong_part (mps_ss_t ss, struct Lisp_Weak_Hash_Table_Strong_Part *t);
@@ -2025,6 +2025,7 @@ dflt_scan_obj (mps_ss_t ss, mps_addr_t base_start, mps_addr_t base_limit,
       case IGC_OBJ_DUMPED_BUFFER_TEXT:
       case IGC_OBJ_DUMPED_BIGNUM_DATA:
       case IGC_OBJ_DUMPED_BYTES:
+      case IGC_OBJ_DUMPED_CHARSET_TABLE:
 	/* Can occur in the dump.  */
 	break;
 
@@ -2080,11 +2081,6 @@ dflt_scan_obj (mps_ss_t ss, mps_addr_t base_start, mps_addr_t base_limit,
 			 fix_blv);
 	break;
 
-      case IGC_OBJ_DUMPED_CHARSET_TABLE:
-	IGC_FIX_CALL (ss, fix_charset_table (ss, (struct charset *)client,
-					     obj_size (header)));
-	break;
-
       case IGC_OBJ_WEAK_HASH_TABLE_STRONG_PART:
 	IGC_FIX_CALL_FN (ss, struct Lisp_Weak_Hash_Table_Strong_Part, client,
 			 fix_weak_hash_table_strong_part);
@@ -2982,8 +2978,8 @@ root_create_bc (struct igc_thread_list *t)
 static void
 root_create_charset_table (struct igc *gc)
 {
-  root_create_ambig (gc, charset_table_init,
-		     charset_table_init + ARRAYELTS (charset_table_init),
+  root_create_exact (gc, &charset_table_init, &charset_table_init + 1,
+		     (mps_area_scan_t) igc_scan_charset_table,
 		     "charset-table");
 }
 
@@ -3069,6 +3065,14 @@ igc_root_create_n (Lisp_Object start[], size_t n)
   return root_create_exact_n (start, n);
 }
 
+void
+igc_root_create_area (void *start, void *end, igc_scan_area_t f, void *closure,
+		      const char *label)
+{
+  root_create (global_igc, start, end, mps_rank_exact (), (mps_area_scan_t) f,
+	       closure, false, label);
+}
+
 static igc_root_list *
 root_find (void *start)
 {
@@ -5150,7 +5154,26 @@ check_dump (mps_addr_t start, mps_addr_t end)
   return true;
 }
 
-static mps_addr_t pinned_objects_in_dump[3];
+static void
+clone_charset (struct charset *src, struct charset *dst)
+{
+  *dst = *src;
+  if (src->code_space_mask)
+    {
+      size_t nbytes = 256;
+      dst->code_space_mask = xzalloc (nbytes);
+      memcpy (dst->code_space_mask, src->code_space_mask, nbytes);
+    }
+}
+
+static void
+clone_charset_table (size_t used, size_t size, struct charset src[size],
+		     struct charset dst[size])
+{
+  igc_assert (global_igc->park_count > 0);
+  for (size_t i = 0; i < used; i++)
+    clone_charset (&src[i], &dst[i]);
+}
 
 /* Called from pdumper_load.  [HOT_START, HOT_END) is the hot section of
    the dump.  [COL_START, COLD_END) is the cold section of the
@@ -5190,20 +5213,13 @@ igc_on_pdump_loaded (void *dump_base, void *hot_start, void *hot_end,
   set_header (hot_end, IGC_OBJ_PAD, discardable_size, 0);
   /* Ignore relocs */
   set_header (heap_end, IGC_OBJ_PAD, relocs_size, 0);
+  /* Copy charset_table out of the GC heap */
+  igc_assert (ARRAYELTS (charset_table_init) == charset_table_size);
+  clone_charset_table (charset_table_used, charset_table_size,
+		       charset_table, charset_table_init);
+  charset_table = charset_table_init;
 
   eassert (check_dump (h, cold_end));
-  /* Pin some stuff in the dump  */
-  mps_addr_t pinned_roots[] = {
-    charset_table,
-    cold_start, /* code_space_masks */
-    cold_user_data_start,
-  };
-  static_assert (sizeof pinned_roots == sizeof pinned_objects_in_dump);
-  memcpy (pinned_objects_in_dump, pinned_roots, sizeof pinned_roots);
-  igc_root_create_ambig (pinned_objects_in_dump,
-			 (uint8_t *) pinned_objects_in_dump
-			   + sizeof pinned_objects_in_dump,
-			 "dump-pins");
 }
 
 void *
diff --git a/src/igc.h b/src/igc.h
index 5e24c7c338a..7275eaf937f 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -152,7 +152,11 @@ #define EMACS_IGC_H
 void igc_root_destroy_comp_unit (struct Lisp_Native_Comp_Unit *u);
 void igc_root_destroy_comp_unit_eph (struct Lisp_Native_Comp_Unit *u);
 void *igc_root_create_n (Lisp_Object start[], size_t n);
+void igc_root_create_area (void *start, void *end, igc_scan_area_t f,
+			   void *closure, const char *label);
 void igc_destroy_root_with_start (void *start);
+igc_scan_result_t igc_scan_charset_table (struct igc_ss *, void *, void *,
+					  void *);
 size_t igc_header_size (void);
 char *igc_dump_finish_obj (void *client, enum igc_obj_type type,
 			   char *base, char *end);
-- 
2.39.5