Re: scratch/igc: Implications of MPS being asynchronous

[Pip Cet]

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

> Pip Cet <pipcet@protonmail.com> writes:
>
>>> I skipped weak structures for learning MPS, so I just left this out. For
>>> MPS, some other weak structure would be needed, If one wants that. Maybe
>>> a weak hash table could be used for the font cache?
>>
>> While weak hash tables may continue to be necessary (I'm not sure about
>> this), for internal data structures we control, finalizers are
>> better:
>>
>> When you allocate a font structure, allocate an extra cons cell which
>> you always use to refer, via XCAR, to the font structure.  Except that
>> in the hash table, you use the car directly.  When the cons cell is no
>> longer reachable (the font structure still is, because it's in the hash
>> table), the finalizer will eventually run, and then you can remove the
>> hash table entry from the hash table, eventually freeing the font
>> structure, too..
>>
>> Note that this works with MPS finalizers (slow) and struct
>> Lisp_Finalizer finalizers (even slower).  We should prefer the former as
>> Lisp finalizers don't behave precisely like MPS finalizers; right now,
>> we just use MPS behavior for Lisp finalizers, but that doesn't match the
>> documentation in every detail.
>
> Something like that, yes. Or maybe just clear the font caches completely
> when some criterion is met? Time, size, something else, no idea.

That's what I meant: we use a dummy sentinel to detect a GC cycle, then
we trigger font cache compaction and re-enable GC cycle detection
afterwards.  We might want to add an additional delay or a flag se we
don't actually do anything in compact_font_caches if we know there
hasn't been a change.

Same thing applies to buffers.

I think it may be confusing that I said "cons cell".  A finalizer PVEC
would be better, but I've picked up the bad habit of using Fcons (Qnil,
Qnil) when I need an EQ-detectable cookie.  Probably from JavaScript,
where {} is the idiom.

Technically, for correctness, we should ensure that an ambiguous pointer
referring to the magic cons cell is never considered a live reference.
The variable is only ever checked for nil-ness or set, never read for
its value.  So that is okay.

I think.

This POC (very dirty, many warnings to remind me of this) prints a
message (only that, the second part is yet to be done), at times when we
think there might be GC pressure.

It may call the function too rarely, but it'll do as a stop-gap (if we
want to finalize font cache entries by using cons cells).

Tested with "(while t (dotimes (i 1000000) (cons nil nil)) (sit-for
0.0))" only, which is atypical usage.

Puresize change necessary to rebuild included for honesty only, bcause I
didn't want to make bootstrap AGAIN.

Pip

diff --git a/src/alloc.c b/src/alloc.c
index 9437bedf650..1fc125704e3 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6348,8 +6348,6 @@ total_bytes_of_live_objects (void)
 
 #ifdef HAVE_WINDOW_SYSTEM
 
-#ifndef HAVE_MPS
-
 /* Remove unmarked font-spec and font-entity objects from ENTRY, which is
    (DRIVER-TYPE NUM-FRAMES FONT-CACHE-DATA ...), and return changed entry.  */
 
@@ -6358,6 +6356,7 @@ compact_font_cache_entry (Lisp_Object entry)
 {
   Lisp_Object tail, *prev = &entry;
 
+#if 0
   for (tail = entry; CONSP (tail); tail = XCDR (tail))
     {
       bool drop = 0;
@@ -6414,21 +6413,16 @@ compact_font_cache_entry (Lisp_Object entry)
       else
        prev = xcdr_addr (tail);
     }
+#endif
   return entry;
 }
 
-#endif // not HAVE_MPS
-
 /* Compact font caches on all terminals and mark
    everything which is still here after compaction.  */
 
-static void
+void
 compact_font_caches (void)
 {
-  // Cannot be done with MPS, which doesn't seem to be tragic
-  // because there is an option to turn it off.
-# ifndef HAVE_MPS
-
   struct terminal *t;
 
   for (t = terminal_list; t; t = t->next_terminal)
@@ -6445,9 +6439,11 @@ compact_font_caches (void)
          for (entry = XCDR (cache); CONSP (entry); entry = XCDR (entry))
            XSETCAR (entry, compact_font_cache_entry (XCAR (entry)));
        }
+#ifndef HAVE_MPS
       mark_object (cache);
-    }
 #endif
+    }
+  fprintf (stderr, "font caches compacted\n");
 }
 
 #else /* not HAVE_WINDOW_SYSTEM */
@@ -6857,7 +6853,7 @@ garbage_collect (void)
      undo lists, and finalizers.  The first two are compacted by
      removing any items which aren't reachable otherwise.  */
 
-  compact_font_caches ();
+  if (0) compact_font_caches ();
 
   FOR_EACH_LIVE_BUFFER (tail, buffer)
     {
diff --git a/src/igc.c b/src/igc.c
index 57c13614f73..81bd270cd1e 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -3398,6 +3398,15 @@ finalize_font (struct font *font)
     }
 }
 
+static void
+finalize_font_sentinel (struct Lisp_Cons *cons)
+{
+  Lisp_Object new_cons = Fcons (Qnil, Qnil);
+  Vfont_cache_gc_flag = new_cons;
+  mps_addr_t ref = XCONS (Vfont_cache_gc_flag);
+  mps_finalize (global_igc->arena, &ref);
+}
+
 static void
 finalize_user_ptr (struct Lisp_User_Ptr *p)
 {
@@ -3604,6 +3613,8 @@ finalize (struct igc *gc, mps_addr_t base)
       finalize_vector (client);
       break;
 
+    case IGC_OBJ_CONS:
+      finalize_font_sentinel (client);
     default:
       break;
     }
@@ -5239,6 +5250,24 @@ syms_of_igc (void)
 were the default value.  */);
   Vigc_step_interval = make_fixnum (0);
 
+  DEFVAR_LISP ("font-cache-gc-flag", Vfont_cache_gc_flag,
+    doc: /* Internal variable for automatic font cache compaction.
+
+Do not use.  It's important that ordinary Lisp code never inspects this
+variable and obtains a reference to its value, but it's necessary for
+special Lisp code to be able to do so.  Failure to do this will nto
+result in catastrophic bugs, but may permanently inhibit automatic font
+cache compaction.
+
+Update: Pip's opinion changed, don't use this at all ever.  This
+variable should only ever be set or checked for nil-ness, never read.*/);
+  Funintern (intern_c_string ("font-cache-gc-flag"), Qnil);
+
+  Vfont_cache_gc_flag = Fcons (Qnil, Qnil);
+  mps_addr_t ref = XCONS (Vfont_cache_gc_flag);
+  mps_finalize (global_igc->arena, &ref);
+  Vfont_cache_gc_flag = Qnil;
+
   DEFVAR_BOOL ("igc--balance-intervals", igc__balance_intervals,
      doc: /* Whether to balance buffer intervals when idle.  */);
   igc__balance_intervals = false;
diff --git a/src/keyboard.c b/src/keyboard.c
index ec166e5ac8f..89215f61c85 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -7903,6 +7903,8 @@ get_input_pending (int flags)
    Return the number of keyboard chars read, or -1 meaning
    this is a bad time to try to read input.  */
 
+void compact_font_caches (void);
+
 int
 gobble_input (void)
 {
@@ -7910,6 +7912,12 @@ gobble_input (void)
   bool err = false;
   struct terminal *t;
 
+  if (!NILP (Vfont_cache_gc_flag))
+    {
+      compact_font_caches ();
+      Vfont_cache_gc_flag = Qnil;
+    }
+
   /* Store pending user signal events, if any.  */
   store_user_signal_events ();
 
diff --git a/src/puresize.h b/src/puresize.h
index f2d00543ee1..35e81eade90 100644
--- a/src/puresize.h
+++ b/src/puresize.h
@@ -47,7 +47,7 @@ #define SITELOAD_PURESIZE_EXTRA 0
 #endif
 
 #ifndef BASE_PURESIZE
-#define BASE_PURESIZE (3100000 + SYSTEM_PURESIZE_EXTRA + 
SITELOAD_PURESIZE_EXTRA)
+#define BASE_PURESIZE (9100000 + SYSTEM_PURESIZE_EXTRA + 
SITELOAD_PURESIZE_EXTRA)
 #endif
 
 /* Increase BASE_PURESIZE by a ratio depending on the machine's word size.  */