Subject: bug#78686: feature/igc: [PATCH] Make charset_table an exact root

From: Helmut Eller <[email-masked]>
Date: Thu, 05 Jun 2025 10:09:12 +0200

On Wed, Jun 04 2025, Pip Cet wrote:

> As far as I can tell, there's just one significant problem: we have a
> root covering charset_table_init even though that array is unused in
> pdumped Emacs sessions.  It's not a huge problem, just 180 zero words
> fixed as Lisp objects, but it is unsatisfying and we should fix it.

It's more like 60kb of zeroes.

> So let's get rid of that unnecessary root.
>
> That leaves two very minor problems:
>
> 1. undumped Emacs sessions.  I don't think these are worth optimizing
> for.  Do you?

No. But there is also no need to make them gratuitously different from
normal sessions.

> 2. Emacs sessions which grow charset_table beyond the 180 entries in the
> dump.  Does this happen?  In any case, we do handle the case correctly,
> leaking memory but giving the user access to as many charsets as they
> desire.

Defining new charsets is presumably an exotic thing to do.

> There is also the long-term goal of making it so all objects in the
> pdump section can be relocated elsewhere, allowing us to free the
> section.  We should do so by tracing objects in the dump precisely so
> they can be moved out.

As my patch does, in contrast to your idea.

[...]
>> Second, I don't like the practice of using the GC header to create
>> "shadow types".
>
> Noted.  That's a good argument for not introducing new ones, but your
> patch doesn't introduce or delete any; it just copies things back from
> the dump heap (which is never freed) to an unprotected root in the BSS
> section or to xmalloc'd memory.

Exactly: my patch copies the charset table to the place where it
belongs, the space in the BSS section reserved for it.  Just like
builtin symbols and subrs are copied to their space in the BSS section.

[...]
>> Maybe, depending on how this charset table business goes.
>
> I don't see the dependency.

My motivation may depend on it.

> As far as I can tell, we need this patch.  It removes the extraneous
> root.

I like my patch better.

Helmut