Subject: bug#79346: igc: PolicyShouldCollectWorld

From: Helmut Eller <[email-masked]>
Date: Thu, 04 Sep 2025 22:12:55 +0200

On Thu, Sep 04 2025, Gerd Möllmann wrote:

[...]
> Anyway, the number and size of generations is such a thing. I've been
> playing with that on and off and I think I found a setting that works
> noticeably smoother than the default.

It seems to me that the arena pause_time parameter must be tuned
together with the size(s) of the generation(s).  Smaller generations
require shorter pause_times.  I gravitate towards 10MB for the first
generation and 10 milliseconds pause time.  My feeling is that something
in the order of 10MB should work better than 100MB.  Of course, this
requires measurements to be sure.

> I've made me something that makes it easier to play with:

I've been using this:

>From 1951f4c7340e4b8ef348f6ffbccc4a1070a90a5d Mon Sep 17 00:00:00 2001
From: Helmut Eller <[email-masked]>
Date: Thu, 6 Feb 2025 18:39:13 +0100
Subject: [PATCH] Add igc_params

---
 src/emacs.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/igc.c   |  28 ++++++++++----
 src/igc.h   |  28 +++++++++++++-
 3 files changed, 155 insertions(+), 10 deletions(-)

diff --git a/src/emacs.c b/src/emacs.c
index 4a46f92f8f8..17e135a2bfa 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -284,6 +284,11 @@ #define MAIN_PROGRAM
 --fingerprint               output fingerprint and exit\n\
 ",
 #endif
+#if HAVE_MPS
+    "\
+--igc-param PARAM=VALUE     set IGC paramater PARAM to VALUE\n\
+"
+#endif
 #if SECCOMP_USABLE
     "\
 --seccomp=FILE              read Seccomp BPF filter from FILE\n\
@@ -723,6 +728,95 @@ argmatch (char **argv, int argc, const char *sstr, const char *lstr,
     }
 }
 
+#ifdef HAVE_MPS
+static void
+parse_igc_gen_params (const char *string, struct igc_params *params)
+{
+  size_t count = 0;
+  struct igc_gen_param *array = NULL;
+  for (const char *s = string;;)
+    {
+      int n = 0;
+      size_t capacity, mortality;
+      if ((sscanf (s, "%zuk%zu%%%n", &capacity, &mortality, &n) == 2)
+	  && mortality <= 100)
+	{
+	  struct igc_gen_param p = { .capacity = capacity,
+				     .mortality = (double)mortality / 100.0 };
+	  count++;
+	  array = xrealloc (array, sizeof (array[0]) * count);
+	  array[count - 1] = p;
+	  if (s[n] == ',')
+	    {
+	      s = &s[n + 1];
+	      continue;
+	    }
+	  else if (s[n] == '\0')
+	    {
+	      params->gen_params.dflt = false;
+	      params->gen_params.gen_count = count;
+	      params->gen_params.gen_params = array;
+	      return;
+	    }
+	  else
+	    break;
+	}
+      else
+	break;
+    }
+  fprintf (stderr, "Invalid value gen_params: %s\n", string);
+  exit (1);
+}
+
+static void
+parse_igc_param (const char *val, struct igc_params *params)
+{
+  int n = 0;
+  if ((sscanf (val, "pause_time=%lf%n", &params->pause_time.seconds, &n) == 1)
+      && val[n] == '\0' && params->pause_time.seconds >= 0)
+    {
+      params->pause_time.dflt = false;
+      return;
+    }
+
+  const char *key = "gen_params=";
+  if (strncmp (val, key, strlen (key)) == 0)
+    return parse_igc_gen_params (val + strlen (key), params);
+
+  if ((sscanf (val, "commit_limit=%zu%n", &params->commit_limit.bytes, &n)
+       == 1)
+      && val[n] == '\0')
+    {
+      params->commit_limit.dflt = false;
+      return;
+    }
+
+  fprintf (stderr, "Invalid igc_param: %s\n", val);
+  exit (1);
+}
+
+static struct igc_params
+parse_igc_params (int argc, char **argv)
+{
+  struct igc_params params = {
+    .pause_time = { .dflt = true },
+    .gen_params = { .dflt = true },
+    .commit_limit = { .dflt = true },
+  };
+  for (int skip = 0; skip != argc;)
+    {
+      char *val = NULL;
+      if (argmatch (argv, argc, "-igc-param", "--igc-param", 11, &val, &skip))
+	parse_igc_param (val, &params);
+      else if (argmatch (argv, argc, "--", NULL, 0, NULL, &skip))
+	break;
+      else
+	skip++;
+    }
+  return params;
+}
+#endif
+
 #if !defined HAVE_ANDROID || defined ANDROID_STUBIFY
 
 /* Find a name (absolute or relative) of the Emacs executable whose
@@ -1368,7 +1462,8 @@ android_emacs_init (int argc, char **argv, char *dump_file)
   init_signals ();
 
 #ifdef HAVE_MPS
-  init_igc ();
+  const struct igc_params params = parse_igc_params (argc, argv);
+  init_igc (&params);
 #endif
 
   /* This is needed early because load_pdump can call 'float-time' (via
@@ -1992,6 +2087,15 @@ android_emacs_init (int argc, char **argv, char *dump_file)
   init_module_assertions (module_assertions);
 #endif
 
+#ifdef HAVE_MPS
+  {
+    char *arg;
+    while (argmatch (argv, argc, "-igc-param", "--igc-param", 11,
+		     &arg, &skip_args))
+      ;
+  }
+#endif
+
 #ifdef HAVE_NS
   if (!noninteractive)
     {
@@ -2615,6 +2719,9 @@ android_emacs_init (int argc, char **argv, char *dump_file)
   { "-no-build-details", "--no-build-details", 63, 0 },
 #ifdef HAVE_MODULES
   { "-module-assertions", "--module-assertions", 62, 0 },
+#endif
+#ifdef HAVE_MPS
+  {"-igc-param", "--igc-param", 61, 1 },
 #endif
   /* -d must come last before the options handled in startup.el.  */
   { "-d", "--display", 60, 1 },
diff --git a/src/igc.c b/src/igc.c
index af73406cecc..94b9a5ee530 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -4643,19 +4643,32 @@ DEFUN ("igc--set-commit-limit", Figc__set_commit_limit,
   return Qnil;
 }
 
+static_assert (sizeof (struct igc_gen_param) == sizeof (mps_gen_param_s));
+
 static void
-make_arena (struct igc *gc)
+make_arena (struct igc *gc, const struct igc_params *params)
 {
   mps_res_t res;
   MPS_ARGS_BEGIN (args)
   {
+    if (!params->pause_time.dflt)
+      MPS_ARGS_ADD (args, MPS_KEY_PAUSE_TIME, params->pause_time.seconds);
+    if (!params->commit_limit.dflt)
+      MPS_ARGS_ADD (args, MPS_KEY_COMMIT_LIMIT, params->commit_limit.bytes);
     res = mps_arena_create_k (&gc->arena, mps_arena_class_vm (), args);
   }
   MPS_ARGS_END (args);
   IGC_CHECK_RES (res);
 
-  mps_gen_param_s gens[] = { { 128000, 0.8 }, { 5 * 128000, 0.4 } };
-  res = mps_chain_create (&gc->chain, gc->arena, ARRAYELTS (gens), gens);
+  mps_gen_param_s dflt_gens[] = { { 128000, 0.8 }, { 5 * 128000, 0.4 } };
+  mps_gen_param_s *gens = dflt_gens;
+  size_t ngens = ARRAYELTS (dflt_gens);
+  if (!params->gen_params.dflt)
+    {
+      ngens = params->gen_params.gen_count;
+      gens = (mps_gen_param_s *)params->gen_params.gen_params;
+    }
+  res = mps_chain_create (&gc->chain, gc->arena, ngens, gens);
   IGC_CHECK_RES (res);
 }
 
@@ -4729,10 +4742,10 @@ make_pool_amcz (struct igc *gc, mps_fmt_t fmt)
 }
 
 static struct igc *
-make_igc (void)
+make_igc (const struct igc_params *params)
 {
   struct igc *gc = xzalloc (sizeof *gc);
-  make_arena (gc);
+  make_arena (gc, params);
 
   /* We cannot let the GC run until at least all staticpros haven been
      processed.  Otherwise we might allocate objects that are not
@@ -5159,11 +5172,11 @@ DEFUN ("igc--remove-extra-dependency", Figc__remove_extra_dependency,
  ***********************************************************************/
 
 void
-init_igc (void)
+init_igc (const struct igc_params *params)
 {
   /* Returns previous handler.  */
   (void) mps_lib_assert_fail_install (igc_assert_fail);
-  global_igc = make_igc ();
+  global_igc = make_igc (params);
   add_main_thread ();
   set_state (IGC_STATE_USABLE_PARKED);
 }
@@ -5174,7 +5187,6 @@ syms_of_igc (void)
   defsubr (&Sigc_info);
   defsubr (&Sigc__roots);
   defsubr (&Sigc__collect);
-  defsubr (&Sigc__set_commit_limit);
   defsubr (&Sigc__add_extra_dependency);
   defsubr (&Sigc__remove_extra_dependency);
   DEFSYM (Qambig, "ambig");
diff --git a/src/igc.h b/src/igc.h
index d4be8e4c03d..89e006948c8 100644
--- a/src/igc.h
+++ b/src/igc.h
@@ -60,8 +60,34 @@ #define EMACS_IGC_H
 
 #ifdef HAVE_MPS
 
+struct igc_gen_param
+{
+  size_t capacity;	  /* in kilobytes */
+  double mortality;	  /* initial estimate; valid values in [0..1] */
+};
+
+struct igc_params
+{
+  struct
+  {
+    bool dflt;
+    double seconds; /* in seconds; valid values in [0..INF] */
+  } pause_time;
+  struct
+  {
+    bool dflt;
+    size_t gen_count;
+    struct igc_gen_param *gen_params;
+  } gen_params;
+  struct
+  {
+    bool dflt;
+    size_t bytes;
+  } commit_limit;
+};
+
 void igc_break (void);
-void init_igc (void);
+void init_igc (const struct igc_params *);
 void syms_of_igc (void);
 void *igc_thread_add (struct thread_state *ts);
 void igc_thread_remove (void **info);
-- 
2.47.2