Re: MPS: hash tables / obarrays

[Helmut Eller <eller.helmut@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 152 bytes --]

Here are some patches to move the remaining parts of hash tables to MPS.
The main advantage is that finalization is not longer needed for hash
tables.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-a-igc_make_byte_vec-function.patch --]
[-- Type: text/x-diff, Size: 1569 bytes --]

From ee5a5dbd42c4a48d9e0cfe4d5c90fb58be89dbea Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Wed, 29 May 2024 10:07:50 +0200
Subject: [PATCH 1/5] Add a igc_make_byte_vec function

This is like igc_make_ptr_vec but for bytes.  The header used is
IGC_OBJ_STRING_DATA.  Perhaps that should be renamed.

* src/igc.h (igc_make_byte_vec): New.
* src/igc.c (igc_make_byte_vec): Implement it.
---
 src/igc.c | 8 ++++++++
 src/igc.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index 4780e68869f..acf8ddbbb4e 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -3054,6 +3054,14 @@ alloc_string_data (size_t nbytes, bool clear)
   return data;
 }
 
+uintptr_t *
+igc_make_byte_vec (size_t nbytes)
+{
+  mps_addr_t addr = alloc (nbytes, IGC_OBJ_STRING_DATA);
+  igc_assert (is_aligned (addr));
+  return (uintptr_t *)addr;
+}
+
 /* Reallocate multibyte STRING data when a single character is
    replaced. The character is at byte offset BYTE_POS in the string.
    The character being replaced is CHAR_LEN bytes long, and the
diff --git a/src/igc.h b/src/igc.h
index 93d27040b6b..bb09210160e 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -106,6 +106,7 @@ #define EMACS_IGC_H
 void *igc_make_ptr_vec (size_t n);
 void *igc_grow_ptr_vec (void *v, ptrdiff_t *n, ptrdiff_t n_incr_min, ptrdiff_t n_max);
 Lisp_Object * igc_make_hash_table_vec (size_t n);
+uintptr_t *igc_make_byte_vec (size_t nbytes); /* result is word aligned */
 struct image_cache *igc_make_image_cache (void);
 struct interval *igc_make_interval (void);
 
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Use-igc_make_byte_vec-for-hash-tables.patch --]
[-- Type: text/x-diff, Size: 3233 bytes --]

From a38b50b84ddd126bf2cdecfce0ba1e878b05c7a9 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Wed, 29 May 2024 10:12:04 +0200
Subject: [PATCH 2/5] Use igc_make_byte_vec for hash tables

This avoids the need for finalization.

* src/alloc.c (hash_table_alloc_bytes, hash_table_free_bytes): Delegate to
igc_make_byte_vec.
* src/igc.c (fix_hash_table): Fix hash, next, and index vectors.
(finalize_hash_table): Delete. No longer needed.
(finalize_vector, maybe_finalize): Remove finalizers for hash tables.
---
 src/alloc.c |  8 ++++++++
 src/igc.c   | 25 +++++--------------------
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 8054deca197..f22e0e8dfb0 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -5794,7 +5794,11 @@ hash_table_alloc_bytes (ptrdiff_t nbytes)
     return NULL;
   tally_consing (nbytes);
   hash_table_allocated_bytes += nbytes;
+#ifdef HAVE_MPS
+  void *p = igc_make_byte_vec (nbytes);
+#else
   void *p = xmalloc (nbytes);
+#endif
   return p;
 }
 
@@ -5804,7 +5808,11 @@ hash_table_free_bytes (void *p, ptrdiff_t nbytes)
 {
   tally_consing (-nbytes);
   hash_table_allocated_bytes -= nbytes;
+#ifdef HAVE_MPS
+  /* let igc handle this */
+#else
   xfree (p);
+#endif
 }
 
 Lisp_Object *
diff --git a/src/igc.c b/src/igc.c
index acf8ddbbb4e..aad7e9d2779 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -1597,6 +1597,9 @@ fix_hash_table (mps_ss_t ss, struct Lisp_Hash_Table *h)
     // FIXME: weak
     IGC_FIX12_RAW (ss, &h->key);
     IGC_FIX12_RAW (ss, &h->value);
+    IGC_FIX12_RAW (ss, &h->hash);
+    IGC_FIX12_RAW (ss, &h->next);
+    IGC_FIX12_RAW (ss, &h->index);
   }
   MPS_SCAN_END (ss);
   return MPS_RES_OK;
@@ -2475,21 +2478,6 @@ igc_create_charset_root (void *table, size_t size)
   root_create_ambig (global_igc, table, (char *) table + size);
 }
 
-static void
-finalize_hash_table (struct Lisp_Hash_Table *h)
-{
-  if (h->table_size)
-    {
-      /* Set the table size to 0 so that we don't further scan a hash
-	 table after it has been finalized. Also, keep in mind that
-	 xfree works with objects in a loaded dump. */
-      h->table_size = 0;
-      xfree (h->index);
-      xfree (h->next);
-      xfree (h->hash);
-    }
-}
-
 static void
 finalize_bignum (struct Lisp_Bignum *n)
 {
@@ -2605,10 +2593,6 @@ finalize_vector (mps_addr_t v)
     case PVEC_FREE:
       emacs_abort ();
 
-    case PVEC_HASH_TABLE:
-      finalize_hash_table (v);
-      break;
-
     case PVEC_BIGNUM:
       finalize_bignum (v);
       break;
@@ -2670,6 +2654,7 @@ finalize_vector (mps_addr_t v)
 #ifndef IN_MY_FORK
     case PVEC_OBARRAY:
 #endif
+    case PVEC_HASH_TABLE:
     case PVEC_SYMBOL_WITH_POS:
     case PVEC_PROCESS:
     case PVEC_RECORD:
@@ -2746,7 +2731,6 @@ maybe_finalize (mps_addr_t client, enum pvec_type tag)
   mps_addr_t ref = client_to_base (client);
   switch (tag)
     {
-    case PVEC_HASH_TABLE:
     case PVEC_BIGNUM:
     case PVEC_FONT:
     case PVEC_THREAD:
@@ -2765,6 +2749,7 @@ maybe_finalize (mps_addr_t client, enum pvec_type tag)
 #ifndef IN_MY_FORK
     case PVEC_OBARRAY:
 #endif
+    case PVEC_HASH_TABLE:
     case PVEC_NORMAL_VECTOR:
     case PVEC_FREE:
     case PVEC_MARKER:
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-No-need-to-fix-the-markers-field-twice.patch --]
[-- Type: text/x-diff, Size: 726 bytes --]

From e382b225ec9de727622ec782597bc47bd2fe2fc0 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Wed, 29 May 2024 10:15:58 +0200
Subject: [PATCH 3/5] No need to fix the markers field twice

* src/igc.c (fix_buffer):
---
 src/igc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/igc.c b/src/igc.c
index aad7e9d2779..591f2fc9f89 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -1478,7 +1478,6 @@ fix_buffer (mps_ss_t ss, struct buffer *b)
     IGC_FIX12_RAW (ss, &b->own_text.intervals);
     IGC_FIX12_RAW (ss, &b->own_text.markers);
     IGC_FIX12_RAW (ss, &b->overlays);
-    IGC_FIX12_RAW (ss, &b->own_text.markers);
 
     IGC_FIX12_RAW (ss, &b->base_buffer);
     if (b->base_buffer)
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-For-igc-info-include-information-about-leaf-objects.patch --]
[-- Type: text/x-diff, Size: 847 bytes --]

From f8554ce726f683f6720ef2676f8904210206e002 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Wed, 29 May 2024 10:18:23 +0200
Subject: [PATCH 4/5] For igc-info, include information about leaf objects

* src/icg.c (Figc_info): Also walk the leaf pool.
---
 src/igc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index 591f2fc9f89..54052375977 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -3295,6 +3295,12 @@ DEFUN ("igc-info", Figc_info, Sigc_info, 0, 0, 0, doc : /* */)
   {
     res = mps_pool_walk (gc->dflt_pool, dflt_scanx, &st);
   }
+  if (res != MPS_RES_OK)
+    error ("Error %d walking memory", res);
+  IGC_WITH_PARKED (gc)
+  {
+    res = mps_pool_walk (gc->leaf_pool, dflt_scanx, &st);
+  }
   if (res != MPS_RES_OK)
     error ("Error %d walking memory", res);
 
-- 
2.39.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-Return-information-about-the-arena-for-igc-info.patch --]
[-- Type: text/x-diff, Size: 1443 bytes --]

From 3ccda8f6174a320544bc8a3752f0d81b8383de2f Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Wed, 29 May 2024 10:21:04 +0200
Subject: [PATCH 5/5] Return information about the arena for igc-info

* src/igc.c (Figc_info): Include mps_arena_committed and other arena
related information.
---
 src/igc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/igc.c b/src/igc.c
index 54052375977..f8df5616020 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -3319,6 +3319,24 @@ DEFUN ("igc-info", Figc_info, Sigc_info, 0, 0, 0, doc : /* */)
 		   make_int (st.pvec[i].nobjs), make_int (st.pvec[i].nwords));
       result = Fcons (e, result);
     }
+  result = Fcons (list2 (build_string ("pause-time"),
+			 make_float (mps_arena_pause_time (gc->arena))),
+		  result);
+  result = Fcons (list2 (build_string ("reserved"),
+			 make_int (mps_arena_reserved (gc->arena))),
+		  result);
+  result = Fcons (list2 (build_string ("spare"),
+			 make_float (mps_arena_spare (gc->arena))),
+		  result);
+  result = Fcons (list2 (build_string ("spare-committed"),
+			 make_int (mps_arena_spare_committed (gc->arena))),
+		  result);
+  result = Fcons (list2 (build_string ("commit-limit"),
+			 make_int (mps_arena_commit_limit (gc->arena))),
+		  result);
+  result = Fcons (list2 (build_string ("committed"),
+			 make_int (mps_arena_committed (gc->arena))),
+		  result);
   return result;
 }
 
-- 
2.39.2