Subject: bug#78898: Make read/readevalloop reentrant

From: Lynn Winebarger <[email-masked]>
Date: Wed, 23 Jul 2025 15:53:51 -0700

On Tue, Jul 22, 2025, 2:40 PM Stefan Monnier <[email-masked]>
wrote:

> > The memory leak aspect is more observation than alarm.  The leak is only
> > safe because the stack is implemented as a global static variable.
> Getting
> > rid of that property means either freeing the stack at the end of the
> > initial entry to read, or allocating the read_entry stack from the
> garbage
> > collected heap.
>
> BTW memory that's not returned to the OS but that Emacs can reuse later
> is generally not considered as a leak.  E.g., most `malloc`
> implementations have the property of (virtually) never bothering to
> return freed memory to the OS.
>

Terminology aside, the memory used for the stack is never freed for use by
other parts of Emacs.  I think the code would be cleaner, and possibly a
little faster, with one of the following approaches:

(1) Just reserve a conservatively large amount of stack space at the start
and abort if it's exceeded.  A lisp variable could be used to let the user
change the size of the reserved space between activations of read0, like
the maximum lisp recursion depth.

(2). Allocate an array in the C stack on entry to read0 for the reader
stack.  During the parse, if all array entries are used, just use C
recursion to call read0.

(3) Reverse the order of call and return in the recursion of option (2).
Meaning, enter read0 with a trampoline that allocates the reader stack with
alloca.  On entry to the read_obj label,  check that there is enough stack
space for the maximum number of entries that can be pushed before control
returns to read_obj.  If there is not enough space, return to the
trampoline.  The trampoline calls read0 in a do...while loop as long as the
stack is nonempty, then returns.  Since the read stack is implemented in a
simple trampoline function, each alloca just extends the initial reader
stack array, so growing that array never requires copying.

I'm going to try implementing (3), since it should be efficient even if
multiple threads were used to run independent lisp VMs.

There's also (3'), in which the trampoline is implemented by putting a
block scope around the parsing code in read0, and putting a trampoline
label just before that block.  That would probably work with -O0, but I
have no idea if the optimizer could reorder code in such a way that the
stack allocations end up being non-contiguous and there are live variable
definitions between them.  It would be nice to replace the "return/call"
sequence with a simple "goto".

> read-from-string is called from the pdump loader when an eln library is
> > initialized.  An occurrence of that notation must be either very unlikely
> > or carefully orchestrated by the pdump loader.
>
> Yup: the \N{CHARNAME} notation cannot occur too early in the build.
> And we know it won't occur at all in the (non-bootstrap) dump because
> the \N{CHARNAME} notation is never used in the `.elc` or `.eln` files
> we generate (because `print.c` never generates it).
>

I'll put this in the "very unlikely" category, at least in configurations
supported by Emacs maintainers.

> Right, all the more reason to have well-defined benchmarking. That's why I
> > stated it the way I did: "time spent in lread.c".  I'm not sure how
> Stefan
> > was measuring the performance of the reader.
>
> I'd suggest you simply concatenate a bunch of `.elc` files, until you
> get something "large enough" to make the timing easy to measure.
> Then measure the time to read that file (put the content inside
> a buffer, wrap it inside a pair of `(...)`, then (benchmark-run
> (read))`).
>

So the circular expression syntax supports creating such nested
expressions?  I didn't notice read0 recurring to the internal start (to get
fresh hash tables for the object map).  I don't see how such an artificial
list of byte code could be guaranteed to be read correctly if those hash
tables are not cleared between top level expressions in the file.

Lynn