Re: MPS: weak hash tables

[Pip Cet <pipcet@protonmail.com>]

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

On Saturday, July 6th, 2024 at 16:31, Gerd Möllmann <gerd.moellmann@gmail.com> wrote:
> Pip Cet pipcet@protonmail.com writes:
> 
> > I wonder how representative that is of actual workloads, though.
> 
> It isn't, for sure :-).
> 
> > It does appear mostly to be the cost of finalization, if I deactivate
> > that (and leak the memory instead!) it takes about 6.5 seconds on this
> > machine compared to ~9 on my main Emacs. Totally unscientific, of
> > course, I'm pretty sure I even updated my compiler since compiling
> > vanilla Emacs. Both use nativecomp.
> 
> I think the same. I under the impression that probably many millions of
> bignums are used, each of which requires inter-thread communication and
> so on. That's quite expensive if true.
> 
> > So IF it's worth optimizing this, the thing to look into would be to
> > use MPS-managed memory for GMP bignums, so we don't need a finalizer
> > at all. I think that's quite easy to do, but requires violating the
> > GMP API by accessing internal member fields directly.
> 
> 
> We have already this in bignum.c
> 
> mp_set_memory_functions (xmalloc, xrealloc_for_gmp, xfree_for_gmp);
> 
> Haven't read the GMP docs yet, but that looks like something that could
> be used.

Patch attached, couldn't resist :-)

Memory usage is perfectly acceptable with this patch, but it does do a lot of GC.

> > I believe Eli is right, though, that we should deviate from what Emacs
> > usually does as little as possible until we've got things working.
> > That means just keeping track of how much we've allocated and calling
> > garbage_collect via maybe_garbage_collect, I think.

> Can you explain? I mean I agree in general, but in this case I'm not so
> sure. What I've currently implemented is basically a function of the
> number of allocated finalizable objects and only finalizable objects pay
> the price. The size of allocated memory overall, i.e. counting the bytes
> in alloc_impl doesn't contain the information.

But it's probably a better indicator of how much GC activity happened, and that influences how many finalization messages there would be, assuming that an approximately constant percentage of objects is finalizable...

Pip

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-mps-bignums.patch --]
[-- Type: text/x-patch; name=0002-mps-bignums.patch, Size: 2762 bytes --]

diff --git a/src/bignum.c b/src/bignum.c
index 1fe195d78ea..330fdf4324c 100644
--- a/src/bignum.c
+++ b/src/bignum.c
@@ -20,12 +20,13 @@
 #include <config.h>
 
 #include "bignum.h"
-
 #include "lisp.h"
 
 #include <math.h>
 #include <stdlib.h>
 
+#include "igc.h"
+
 /* mpz global temporaries.  Making them global saves the trouble of
    properly using mpz_init and mpz_clear on temporaries even when
    storage is exhausted.  Admittedly this is not ideal.  An mpz value
@@ -34,18 +35,29 @@
    rounddiv_q and rounding_driver both need four and time_arith needs
    five.  */
 
-mpz_t mpz[5];
+mpz_t mpz[5] = { 0, };
+
+static void *
+xmalloc_for_gmp (size_t size)
+{
+  return igc_alloc_bytes (size);
+}
 
 static void *
 xrealloc_for_gmp (void *ptr, size_t ignore, size_t size)
 {
-  return xrealloc (ptr, size);
+  if (size > ignore)
+    {
+      void *ret = xmalloc_for_gmp (size);
+      memcpy (ret, ptr, ignore);
+      return ret;
+    }
+  return ptr;
 }
 
 static void
 xfree_for_gmp (void *ptr, size_t ignore)
 {
-  xfree (ptr);
 }
 
 void
@@ -62,8 +74,10 @@ init_bignum (void)
      program execution.  A 'longjmp' or throwing a C++ exception will
      have undefined results."  But xmalloc and xrealloc do call
      'longjmp'.  */
-  mp_set_memory_functions (xmalloc, xrealloc_for_gmp, xfree_for_gmp);
+  mp_set_memory_functions (xmalloc_for_gmp, xrealloc_for_gmp, xfree_for_gmp);
 
+  for (int i = 0; i < ARRAYELTS (mpz); i++)
+    igc_root_create_exact_ptr (&mpz[i][0]._mp_d);
   for (int i = 0; i < ARRAYELTS (mpz); i++)
     mpz_init (mpz[i]);
 }
diff --git a/src/igc.c b/src/igc.c
index ccfa136d258..f2fa3270765 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -2170,6 +2170,19 @@ fix_marker (mps_ss_t ss, struct Lisp_Marker *m)
   return MPS_RES_OK;
 }
 
+static mps_res_t
+fix_bignum (mps_ss_t ss, struct Lisp_Bignum *m)
+{
+  MPS_SCAN_BEGIN (ss)
+  {
+    mps_addr_t ref = m->value[0]._mp_d;
+    IGC_FIX12_RAW (ss, &ref);
+    m->value[0]._mp_d = ref;
+  }
+  MPS_SCAN_END (ss);
+  return MPS_RES_OK;
+}
+
 static mps_res_t
 fix_finalizer (mps_ss_t ss, struct Lisp_Finalizer *f)
 {
@@ -2375,6 +2388,7 @@ fix_vector (mps_ss_t ss, struct Lisp_Vector *v)
 	break;
 
       case PVEC_BIGNUM:
+	IGC_FIX_CALL_FN (ss, struct Lisp_Bignum, v, fix_bignum);
 	break;
 
       case PVEC_NATIVE_COMP_UNIT:
@@ -3195,7 +3209,6 @@ maybe_finalize (mps_addr_t client, enum pvec_type tag)
   mps_addr_t ref = client_to_base (client);
   switch (tag)
     {
-    case PVEC_BIGNUM:
     case PVEC_FONT:
     case PVEC_THREAD:
     case PVEC_MUTEX:
@@ -3210,6 +3223,7 @@ maybe_finalize (mps_addr_t client, enum pvec_type tag)
       mps_finalize (global_igc->arena, &ref);
       break;
 
+    case PVEC_BIGNUM:
 #ifndef IN_MY_FORK
     case PVEC_OBARRAY:
 #endif