Re: MPS: w32 threads

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 05 May 2024 11:16:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Let's say we made a thread_state for GUI thread. If we had that, you
> could simply call this:
> 
> static struct igc_thread_list *
> thread_add (struct thread_state *ts)
> {
>   mps_thr_t thr;
>   mps_res_t res = mps_thread_reg (&thr, global_igc->arena);
>   IGC_CHECK_RES (res);
>   struct igc_thread_list *t = register_thread (global_igc, thr, ts);
>   root_create_thread (t);
>   create_thread_aps (&t->d);
>   return t;
> }

thread_state is HUGE!  Almost everything there is unneeded for the
other threads, which are not Lisp threads, they run straightforward
and quite simple C code.  So I'd like to avoid that if possible, which
is why I asked all those questions.

> > 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?
> 
> What's currently there is simply the result of me reading the docs, and
> picking what I thought would be needed. I guess there are a lot of knobs
> that could be tried... That is also the case for pools, generations....

I'd appreciate if you could add at least some comments about why you
thought this stuff and not the rest.

> > Assuming we can do with the single global arena already defined, 
> 
> There is no need for an additional arena.

OK, thanks.

> > 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.
> 
> The _scanned variant allows us to use our own scan method, which is
> necessary because how we represent symbols in Lisp_Objects (the NULL ==
> nil thing.)

Got it, thanks.

> >   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?
> 
> I'm not sure I have understood how that works. Is it like in pthread
> that you create a thread specifying a thread_main? And when you return
> from thread_main the thread is over?

Yes.  Example:

          cp->thrd = CreateThread (NULL, 64 * 1024, reader_thread, cp,
                                   0x00010000, &id);

where reader_thread is the thread worker function.  When the thread's
work is done, that function exits, and the thread exits as well.

My question is whether I can make the mps_root_create_thread call from
inside reader_thread and other similar worker functions, or do I need
to go through an intermediary, like so:

  static int
  worker (...)
  {
    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");
    return real_worker ();
  }

  CreateThread (..., worker, ...);

> >   . igc_thread_list_push (what is it? and where defined?)
> >   . igc_root_list_push (where is it defined and what does it do?)
> 
> It's a very ppor man's alternative to C++ std::list, implemented with
> abominable macros, see IGC_DEFINE_LIS.

I reckon we don't need to keep the threads I'm talking about on a
list, since they don't have any specpdl pointers?

> >   . create_thread_aps (what is it for?)
> 
> These are allocation points for allocating from MPS. Each thread needs
> its own aps. (It wouldn't hurt to have aps, even if they are not used.)

OK, but what is each of those 4 aps for? are all of them needed for a
thread that doesn't run Lisp?  I see these aps are stored in struct
igc_thread, which seems to be for Lisp threads?

> In summary, I would really recommend, if technically possible on Window,
> to set up a thread_state and use what's there in igc.c.

I'd like to avoid that, for reasons explained above.

Thanks.