MPS: hash tables / obarrays

MPS: hash tables / obarrays

[Gerd Möllmann]

I have pushed a change that allocates key/value vectors for hash tables
from MPS, something that had to be done at some point to gain exclusive
access to these vectors when scanning them.

Same is true for obarrays.

I think obarrays in the dump where not made exact roots before. so this
should work much better than before.

If someone could check the obarray related changes I made, that would be
nice.

Re: MPS: hash tables / obarrays

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Sun, 19 May 2024 10:38:53 +0200
> 
> I have pushed a change that allocates key/value vectors for hash tables
> from MPS, something that had to be done at some point to gain exclusive
> access to these vectors when scanning them.
> 
> Same is true for obarrays.
> 
> I think obarrays in the dump where not made exact roots before. so this
> should work much better than before.
> 
> If someone could check the obarray related changes I made, that would be
> nice.

Any suggestions for how to test that?

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> If someone could check the obarray related changes I made, that would be
>> nice.
>
> Any suggestions for how to test that?

No good ones, I must admit.

Before my change, and after we made the dump exact roots, I had spurious
errors like bus errors, assertion failures and alike that hinted at
symbols being freed. Always something involving symbols.

Maybe if you run the test suite a couple of times, and it survives that?

Re: MPS: hash tables / obarrays

[Helmut Eller]

On Sun, May 19 2024, Gerd Möllmann wrote:

> If someone could check the obarray related changes I made, that would be
> nice.

Does this only work if the dump is registered as exact root or also with
ambiguous roots?

Because for me, comp-tests now fails reliably.  First I thought this has
something to do with the dump being an exact root, but when I tried with
ambiguous roots and discovered that comp-tests fails even faster.
Basically immediately, while with exact roots it takes 2-3 minutes.

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Sun, May 19 2024, Gerd Möllmann wrote:
>
>> If someone could check the obarray related changes I made, that would be
>> nice.
>
> Does this only work if the dump is registered as exact root or also with
> ambiguous roots?

I only works with exact roots ATM. We could support both by changing
igc_make_hash_table_vec to add a root for the vector for hash tables
that are in the dump. So, the same that was in hash_table_alloc_kv
before.

>
> Because for me, comp-tests now fails reliably.  First I thought this has
> something to do with the dump being an exact root, but when I tried with
> ambiguous roots and discovered that comp-tests fails even faster.
> Basically immediately, while with exact roots it takes 2-3 minutes.

Alas, I can't currently configure with native-comp because something in
the libgccjit I got from Homebrew changed. It can't find libgcc_s.dylib
anymore.

What does it say when it fails?

Re: MPS: hash tables / obarrays

[Helmut Eller]

On Sun, May 19 2024, Gerd Möllmann wrote:

> What does it say when it fails?

I think it prints a non-interned symbol.  print-gensym seems to be t.
With exact roots:

gdb --args ../src/emacs -Q --batch -l src/comp-tests.el --eval
'(ert-run-tests-batch "comp-tests-bootstrap")'

...
(gdb) ba 20
#0  __pthread_kill_implementation
    (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0)
    at ./nptl/pthread_kill.c:44
#1  0x00007ffff2ca9e8f in __pthread_kill_internal
    (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  0x00007ffff2c5afb2 in __GI_raise (sig=6) at ../sysdeps/posix/raise.c:26
#3  0x00005555557637a1 in terminate_due_to_signal
    (sig=6, backtrace_limit=2147483647) at emacs.c:480
#4  0x00005555559014c0 in igc_assert_fail
    (file=0x555555a03852 "igc.c", line=343, msg=0x555555a0387a "h->obj_type != IGC_OBJ_FWD") at igc.c:90
#5  0x00005555559016e6 in igc_check_fwd (client=0x7fffe2e7f268) at igc.c:343
#6  0x00005555558777a3 in XBARE_SYMBOL (a=0x2aaa8cd77588)
    at /scratch/emacs/emacs-igc/src/lisp.h:1166
#7  0x000055555587781b in XSYMBOL (a=0x2aaa8cd77588)
    at /scratch/emacs/emacs-igc/src/lisp.h:1179
#8  0x0000555555878319 in SYMBOL_INTERNED_P (sym=0x2aaa8cd77588)
    at /scratch/emacs/emacs-igc/src/lisp.h:2413
#9  0x000055555587f60f in print_object
    (obj=0x2aaa8cd77588, printcharfun=0x0, escapeflag=true) at print.c:2271
#10 0x00005555558809db in print_object
    (obj=0x7fffe3d24b4d, printcharfun=0x0, escapeflag=true) at print.c:2608
#11 0x000055555587ca32 in print
    (obj=0x7fffe253cc93, printcharfun=0x0, escapeflag=true) at print.c:1296
#12 0x000055555587b907 in Fprin1_to_string
    (object=0x7fffe253cc93, noescape=0x0, overrides=0x0) at print.c:814
#13 0x0000555555846ef1 in funcall_subr
    (subr=0x55555608a1e0 <Sprin1_to_string>, numargs=1, args=0x7fffde3b76c0)
    at eval.c:3104
#14 0x00005555558a4bd8 in exec_byte_code
    (fun=0x7fffeb74ab35, args_template=257, nargs=1, args=0x7fffffffaf70)
    at bytecode.c:829
#15 0x000055555584759f in funcall_lambda
    (fun=0x7fffeb74ab35, nargs=1, arg_vector=0x7fffffffaf68) at eval.c:3200
#16 0x000055555584688a in funcall_general
    (fun=0x7fffeb74ab35, numargs=1, args=0x7fffffffaf68) at eval.c:2982
#17 0x0000555555846b67 in Ffuncall (nargs=2, args=0x7fffffffaf60)
    at eval.c:3032
#18 0x0000555555858dfe in mapcar1
    (leni=5, vals=0x0, fn=0x7fffeb74ab35, seq=0x7fffe253cd6b) at fns.c:3346
#19 0x0000555555859611 in Fmapc
    (function=0x7fffeb74ab35, sequence=0x7fffe253cd6b) at fns.c:3483
(More stack frames follow...)
(gdb) p is_mps(0x2aaa8cd77588)
$1 = false
(gdb) p is_mps(0x7fffe2e7f268)
$2 = true

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Sun, May 19 2024, Gerd Möllmann wrote:
>
>> What does it say when it fails?
>
> I think it prints a non-interned symbol.  print-gensym seems to be t.
> With exact roots:
>
> gdb --args ../src/emacs -Q --batch -l src/comp-tests.el --eval
> '(ert-run-tests-batch "comp-tests-bootstrap")'
>
> ...
>     (file=0x555555a03852 "igc.c", line=343, msg=0x555555a0387a "h->obj_type != IGC_OBJ_FWD") at igc.c:90
> #5  0x00005555559016e6 in igc_check_fwd (client=0x7fffe2e7f268) at igc.c:343
> #6  0x00005555558777a3 in XBARE_SYMBOL (a=0x2aaa8cd77588)
>     at /scratch/emacs/emacs-igc/src/lisp.h:1166
> #7  0x000055555587781b in XSYMBOL (a=0x2aaa8cd77588)
>     at /scratch/emacs/emacs-igc/src/lisp.h:1179
> #8  0x0000555555878319 in SYMBOL_INTERNED_P (sym=0x2aaa8cd77588)

> (gdb) p is_mps(0x2aaa8cd77588)
> $1 = false
> (gdb) p is_mps(0x7fffe2e7f268)
> $2 = true

I'd say a reference we didn't scan :-(.

The is_mps with a Lisp_Object doesn't work with symbols, BTW, because
the Lisp_Object doesn't contain a pointer, but an offset from lispsym.

Re: MPS: hash tables / obarrays

[Helmut Eller]

> I'd say a reference we didn't scan :-(.
>
> The is_mps with a Lisp_Object doesn't work with symbols, BTW, because
> the Lisp_Object doesn't contain a pointer, but an offset from lispsym.

The function that tries to print is comp--final and the argument is the
expr.  When I call safe_debug_print on that it goes on for many pages
and ends with this:

   #2314# 635 #2194# 636 #2181# 637 #2022# 638 #1949# 639 #1891# 640 #1640#)) #s(comp-data-container nil #s(hash-table test 
  igc.c:343: Emacs fatal error: assertion failed: h->obj_type != IGC_OBJ_FWD
  
  Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
      at emacs.c:443
  443       signal (sig, SIG_DFL);
  The program being debugged stopped while in a function called from GDB.
  Evaluation of the expression containing the function
  (safe_debug_print) will be abandoned.
  When the function is done executing, GDB will silently stop.
  (gdb)

So it could have something to do with hash tables after all.

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

>> I'd say a reference we didn't scan :-(.
>>
>> The is_mps with a Lisp_Object doesn't work with symbols, BTW, because
>> the Lisp_Object doesn't contain a pointer, but an offset from lispsym.
>
> The function that tries to print is comp--final and the argument is the
> expr.  When I call safe_debug_print on that it goes on for many pages
> and ends with this:
>
>    #2314# 635 #2194# 636 #2181# 637 #2022# 638 #1949# 639 #1891# 640 #1640#)) #s(comp-data-container nil #s(hash-table test 
>   igc.c:343: Emacs fatal error: assertion failed: h->obj_type != IGC_OBJ_FWD
>   
>   Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
>       at emacs.c:443
>   443       signal (sig, SIG_DFL);
>   The program being debugged stopped while in a function called from GDB.
>   Evaluation of the expression containing the function
>   (safe_debug_print) will be abandoned.
>   When the function is done executing, GDB will silently stop.
>   (gdb)
>
> So it could have something to do with hash tables after all.

Could be, of course.

Maybe you could try the following: In igc_check_fwd, where we find the
header has IGC_OBJ_FWD,

  void
  igc_check_fwd (void *client)
  {
    if (is_mps (client))
      {
        struct igc_header *h = client_to_base (client);
        igc_assert (h->obj_type != IGC_OBJ_FWD);
      }
  }

  struct igc_fwd
  {
    struct igc_header header;
    mps_addr_t new_base_addr;
  };

When h->obj_type == IGC_OBJ_FWD, then the header is actually an igc_fwd,
and the new_base_addr points to where the object was forwarded to.
dflt_fwd does that. Some phantasy debug commands woould look like

(lldb) p *(struct igc_fwd *) h
=> ...new_base_addr = 0x1234

(lldb) p *(struct igc_header *) 0x1234
(lldb) p base_to_client ((void *) 0x1234)

We could then perhaps, not always, see what the real object looks like.
For example, if it is a symbol, its name etc. Maybe that gives us a
clue.

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>>> I'd say a reference we didn't scan :-(.
>>>
>>> The is_mps with a Lisp_Object doesn't work with symbols, BTW, because
>>> the Lisp_Object doesn't contain a pointer, but an offset from lispsym.
>>
>> The function that tries to print is comp--final and the argument is the
>> expr.  When I call safe_debug_print on that it goes on for many pages
>> and ends with this:
>>
>>    #2314# 635 #2194# 636 #2181# 637 #2022# 638 #1949# 639 #1891# 640 #1640#)) #s(comp-data-container nil #s(hash-table test
>>   igc.c:343: Emacs fatal error: assertion failed: h->obj_type != IGC_OBJ_FWD
>>
>>   Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
>>       at emacs.c:443
>>   443       signal (sig, SIG_DFL);
>>   The program being debugged stopped while in a function called from GDB.
>>   Evaluation of the expression containing the function
>>   (safe_debug_print) will be abandoned.
>>   When the function is done executing, GDB will silently stop.
>>   (gdb)
>>
>> So it could have something to do with hash tables after all.
>
> Could be, of course.
>
> Maybe you could try the following: In igc_check_fwd, where we find the
> header has IGC_OBJ_FWD,
>
>   void
>   igc_check_fwd (void *client)
>   {
>     if (is_mps (client))
>       {
>         struct igc_header *h = client_to_base (client);
>         igc_assert (h->obj_type != IGC_OBJ_FWD);
>       }
>   }
>
>   struct igc_fwd
>   {
>     struct igc_header header;
>     mps_addr_t new_base_addr;
>   };
>
> When h->obj_type == IGC_OBJ_FWD, then the header is actually an igc_fwd,
> and the new_base_addr points to where the object was forwarded to.
> dflt_fwd does that. Some phantasy debug commands woould look like
>
> (lldb) p *(struct igc_fwd *) h
> => ...new_base_addr = 0x1234
>
> (lldb) p *(struct igc_header *) 0x1234
> (lldb) p base_to_client ((void *) 0x1234)
>
> We could then perhaps, not always, see what the real object looks like.
> For example, if it is a symbol, its name etc. Maybe that gives us a
> clue.

Probably unrelated, but since we're talking native-comp... I've just
removed the special handling of comp units in the dump. This should be
unnecessary now that we traverse the dump, and it's actually against the
MPS rule that roots may not overlap...

Re: MPS: hash tables / obarrays

[Helmut Eller]

On Mon, May 20 2024, Gerd Möllmann wrote:

> Helmut Eller <eller.helmut@gmail.com> writes:
>>    #2314# 635 #2194# 636 #2181# 637 #2022# 638 #1949# 639 #1891# 640 #1640#)) #s(comp-data-container nil #s(hash-table test 
>>
>> So it could have something to do with hash tables after all.
>
> Could be, of course.

The hash table test wasn't scanned.  Here is a test to show it:

(progn
  (progn
    (defconst foo-sym (make-symbol "foo"))
    (define-hash-table-test foo-sym (function eql) (function sxhash-eql))
    (defconst foo-var (make-hash-table :test foo-sym)))
  (igc--collect)
  (message "%s" foo-var))

And with this it works:

diff --git a/src/igc.c b/src/igc.c
index fd206a0ff01..362c0cc32da 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -1518,6 +1518,11 @@ fix_hash_table (mps_ss_t ss, struct Lisp_Hash_Table *h)
     // FIXME: weak
     IGC_FIX12_NOBJS (ss, h->key, h->table_size);
     IGC_FIX12_NOBJS (ss, h->value, h->table_size);
+    const struct hash_table_test *p = h->test;
+    struct hash_table_test *test = (struct hash_table_test *)p;
+    IGC_FIX12_OBJ (ss, &test->user_hash_function);
+    IGC_FIX12_OBJ (ss, &test->user_cmp_function);
+    IGC_FIX12_OBJ (ss, &test->name);
   }
   MPS_SCAN_END (ss);
   return MPS_RES_OK;

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Mon, May 20 2024, Gerd Möllmann wrote:
>
>> Helmut Eller <eller.helmut@gmail.com> writes:
>>>    #2314# 635 #2194# 636 #2181# 637 #2022# 638 #1949# 639 #1891# 640 #1640#)) #s(comp-data-container nil #s(hash-table test 
>>>
>>> So it could have something to do with hash tables after all.
>>
>> Could be, of course.
>
> The hash table test wasn't scanned.  Here is a test to show it:

Oops ;-). Thanks Helmut and pushed.

Re: MPS: hash tables / obarrays

[Helmut Eller]

[-- 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

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> 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.

Thanks, and pushed.

BTW, I've made another attempt to get a grip on the native build on
arm64/macOS. No success. Strangely, when I built with -lmps instead of
-lmps-debug that worked.

Do you build with -lmps-debug?

Re: MPS: hash tables / obarrays

[Helmut Eller]

On Wed, May 29 2024, Gerd Möllmann wrote:

> BTW, I've made another attempt to get a grip on the native build on
> arm64/macOS. No success. Strangely, when I built with -lmps instead of
> -lmps-debug that worked.
>
> Do you build with -lmps-debug?

Yes, I always used the debug version.  Have you tried to the make the
entire rdstack and the prstack ambiguous roots?  Or perhaps there's
another simple way to rule out bugs with reading/printing?

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Wed, May 29 2024, Gerd Möllmann wrote:
>
>> BTW, I've made another attempt to get a grip on the native build on
>> arm64/macOS. No success. Strangely, when I built with -lmps instead of
>> -lmps-debug that worked.
>>
>> Do you build with -lmps-debug?
>
> Yes, I always used the debug version.  Have you tried to the make the
> entire rdstack and the prstack ambiguous roots?  Or perhaps there's
> another simple way to rule out bugs with reading/printing?

I've made the specbindings and bytecode stack ambiguous roots in their
entirety with no effect whatsoever.

The rd and pr stacks I've left alone because they don't really fit what
little I could see. One example, native-compiled:

(defun byte-compile-eval (form)
  "Eval FORM and mark the functions defined therein.
Each function's symbol gets added to `byte-compile-noruntime-functions'."
  (let ((hist-orig load-history)
	(hist-nil-orig current-load-list))
    (prog1 (eval form lexical-binding)
      (when (byte-compile-warning-enabled-p 'noruntime)
	(let* ((hist-new
	        ;; Get new `current-load-list' for the locally defined funs.
	        (cons (butlast current-load-list
	                       (length hist-nil-orig))
	              load-history)))

Here length found that the first cons of the hist-nil-orig has been
forwarded. And the C code generated for soeed 0 and 1 is almost
identical. 0 works, 1 doesn't. And to add to that when I pass -O1 plus
all the -fxy options -O1 says it consists of as -fno-xy it still fails.

And so on and so on :-/,

BTW, just saw an hour ago that SAFE_ALLOCA_LISP can xzalloc an array of
Lisp_Objects. Nice nice :-/. I guess I'll put an emacs_abort there. Not
that it will help.

Re: MPS: hash tables / obarrays

[Andrea Corallo]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>> On Wed, May 29 2024, Gerd Möllmann wrote:
>>
>>> BTW, I've made another attempt to get a grip on the native build on
>>> arm64/macOS. No success. Strangely, when I built with -lmps instead of
>>> -lmps-debug that worked.
>>>
>>> Do you build with -lmps-debug?
>>
>> Yes, I always used the debug version.  Have you tried to the make the
>> entire rdstack and the prstack ambiguous roots?  Or perhaps there's
>> another simple way to rule out bugs with reading/printing?
>
> I've made the specbindings and bytecode stack ambiguous roots in their
> entirety with no effect whatsoever.
>
> The rd and pr stacks I've left alone because they don't really fit what
> little I could see. One example, native-compiled:
>
> (defun byte-compile-eval (form)
>   "Eval FORM and mark the functions defined therein.
> Each function's symbol gets added to `byte-compile-noruntime-functions'."
>   (let ((hist-orig load-history)
> 	(hist-nil-orig current-load-list))
>     (prog1 (eval form lexical-binding)
>       (when (byte-compile-warning-enabled-p 'noruntime)
> 	(let* ((hist-new
> 	        ;; Get new `current-load-list' for the locally defined funs.
> 	        (cons (butlast current-load-list
> 	                       (length hist-nil-orig))
> 	              load-history)))
>
> Here length found that the first cons of the hist-nil-orig has been
> forwarded. And the C code generated for soeed 0 and 1 is almost
> identical. 0 works, 1 doesn't.

Hi Gerd,

in case you need you can decouple the optimizations we do un our
compiler from the GCC ones forcing the number you want in our call to
'gcc_jit_context_set_int_option' in comp.c.

That said if you see almost no differences in the pseudo C we ask GCC to
compile you've probably got already the answer.

Anyway IMO the big diff from -O0 -O1 here should be that values are not
constantly loaded and stored into the stack, this might indeed make a
difference on the GC.

  Andrea

Re: MPS: hash tables / obarrays

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> in case you need you can decouple the optimizations we do un our
> compiler from the GCC ones forcing the number you want in our call to
> 'gcc_jit_context_set_int_option' in comp.c.

Good idea.

> That said if you see almost no differences in the pseudo C we ask GCC to
> compile you've probably got already the answer.
>
> Anyway IMO the big diff from -O0 -O1 here should be that values are not
> constantly loaded and stored into the stack, this might indeed make a
> difference on the GC.

Thanks. That matches what I can "see" in the raw assembler code in LLDB.

(Haven't looked yet at the jit code in the repo BTW to find out how I
can keep the the .o or .s files. Seems I can't compile GCC anyway
without jumping through hoops, on my system :-/).

And then... I saw by accident that it builds with -lmps instead of
-lmps-debug. That finished me off :-(.