Re: MPS: w32 threads

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: eller.helmut@gmail.com,  luangruo@yahoo.com,  emacs-devel@gnu.org
> Date: Sat, 04 May 2024 16:20:19 +0200
> 
> >> A thread must be registered with an arena if:
> >> 
> >> its control stack and registers form a root (this is enforced by
> >> mps_root_create_thread()); or it reads or writes from a location in an
> >> automatically managed pool in the arena. However, some automatically
> >> managed pool classes may be more liberal than this. See the
> >> documentation for the pool class.
> >
> > Can you try translating this to Emacs-related terms, please?  What
> > activities in the thread should require the thread to be "registered
> > with an arena"?
> 
> It basically says that if a thread can have Lisp_Objects or pointers to
> them on its control stack, register it with MPS.
> 
> An arena is basically an implementation of some low-level aspects of
> memory managemnt in MPS. We have only one MPS arena, and it is one using
> VM functions (mmap, ...).
> 
> The hint that some pool classes don't require this or that we can
> ignore. AKAIK the mostly-copying pools we use require it.
> 
> > Emacs on Windows starts separate threads for the following jobs:
> >
> >   . processing Windows GUI-related messages
> >   . reading from sub-processes (part of pselect emulation)
> >   . emulation of itimers (for atimers and for M-x profiler)
> >   . file-change notifications
> >
> > I need to understand what traits of a thread make it need to be
> > registered with MPS.  Each of the above will then need a separate
> > audit, I think.
> 
> Using any Lisp_Objects (or other objects allocated from MPS) -> register
> it. AFAIU, the GUI thread gets such an objects posted in a message, so
> that one must be registered.
> 
> The others are hard to say for me. It would not do harm to register
> them, but might mean addtional work for nothing.

So how does one go about registering a thread?

I've just got up from a full hour reading the seemingly-related parts
of igc.c and the MPS documentation (the latter, I must say, is really
badly written and organized illogically), and I still have quite a few
questions about this.

First, the MPS manual says to call mps_thread_reg from the thread
itself, to register a thread.  That function accepts the ARENA
argument.  I see we have a single global arena, but should other
threads use the same arena, or should they create their own?  The
considerations for that are not clear (are they documented
somewhere?), I only see that you used the same arena for the Lisp
threads in thread_add.  Is that arena defined in a way that could
serve additional threads, perhaps many of them?  (The w32 build starts
a separate thread for any file-notification watch, up to 64 threads
for sub-process communications, and one thread per atimer, so in
principle we could have quite a few of them, if a session is a heavy
user of these features.)

While trying to think about the need for an additional arena, I took a
look at the relevant APIs, and became confused with what we do (or
don't do) in make_arena.  All the keywords we add  with MPS_ARGS_ADD
and the magic constants there -- what do they mean and what were the
considerations to choose them and not the others?  Can some commentary
be added there to explain what we need to know, and maybe even mention
optional features we might consider in the future?

Assuming we can do with the single global arena already defined, the
MPS documentation next says that every thread's registers and stack
should be registered using mps_root_create_thread.  But the code in
igc.c uses mps_root_create_thread_scanned instead, whose documentation
is obscure, and I cannot figure out whether to copycat or do something
else.

Next, mps_root_create_thread and mps_root_create_thread_scanned
require a COLD pointer, about which the MPS documentation says:

  The MPS’s stack scanner needs to know how to find the cold end of the
  part of the stack to scan.  The cold end of the relevant part of the
  stack can be found by taking the address of a local variable in the
  function that calls the main work function of your thread.  You should
  take care to ensure that the work function is not inlined so that the
  address is definitely in the stack frame below any potential roots.

But in our case, the thread function is run directly by the MS-Windows
CreateThread API, not via an intermediate function, so the worker
function is effectively "inlined".  So is it okay to have the cold end
marker be a local variable in the same thread-worker function?  In
another place of the MPS documentation there's an example which seems
to answer in the positive:

     void *marker = &marker;
     mps_root_t stack_root;
     res = mps_root_create_thread(&stack_root, arena, thread, marker);
     if (res != MPS_RES_OK) error("Couldn't create stack root");

Am I missing something, or could the above be at the very beginning of
a thread's worker function?  Or do we have to define intermediary
functions for each thread worker?

mps_root_create_thread_scanned also takes the CLOSURE argument, which
is a pointer, and we pass a NULL pointer as its value(??).  Is this
because our scan_area function, scan_ambig, simply ignores the CLOSURE
argument?

What else needs to be done for "registering a thread"?  igc_thread_add
does a few other things; some of them are Lisp-related (so not
relevant for non-Lisp threads), but others aren't, so I'm unsure about
these:

  . igc_thread_list_push (what is it? and where defined?)
  . igc_root_list_push (where is it defined and what does it do?)
  . create_thread_aps (what is it for?)

I think some minimal commentary to what these functions do is in
order, to make the code look less like some "black magic".

TIA