SIGPROF + SIGCHLD and igc

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

(I've given this a new subject.)

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

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Cc: Eli Zaretskii <eliz@gnu.org>,  ofv@wanadoo.es,  emacs-devel@gnu.org,
>>>   eller.helmut@gmail.com,  acorallo@gnu.org
>>> Date: Mon, 23 Dec 2024 18:44:42 +0100
>>> 
>>> BTW, do you know which signal handlers use Lisp, i.e. allocate Lisp
>>> objects or access some? All? Or, would it be realistic to rewrite signal
>>> handlers to not do that?
>>
>> SIGPROF does (it's the basis for our Lisp profiler).
>>
>> SIGCHLD doesn't run Lisp (I think), but it examines objects and data
>> structures of the Lisp machine (those related to child processes).
>>
>>> One thing I've seen done elsewhere is to publish a message to a message
>>> board so that it can be handled outside of the signal handler. Something
>>> like that, you know what I mean.
>>
>> This is tricky for the profiler, because you want to sample the
>> function in which you are right there and then, not some time later.
>>
>> For SIGCHLD this could work, but it might make Emacs slower in
>> handling subprocesses (there are some Lisp packages that fire
>> subprocesses at very high rate).
>
> Thanks.
>
> I've looked at SIGPROF. From an admittedly brief look at this, I'd
> summarize my results as:
>
> - The important part is get_backtrace. The rest could be done elsewhere
>   by posting that to a message board, or whatever the mechanism is at
>   the end.
>
> - Didn't see get_backtrace or functions called from it allocating Lisp
>   objects.
>
> - It reads from a Lisp object because of
>
>     #define specpdl (current_thread->m_specpdl)
>     #define specpdl_end (current_thread->m_specpdl_end)
>     #define specpdl_ptr (current_thread->m_specpdl_ptr)
>
>   current_thread is a struct thread_state which is a PVEC_THREAD.
>
> - I remember that I wrote a scanner for the specpdl stacks, so that's
>   not a Lisp object but a root, so no problem here, I think.
>
> - struct thread_state allocation is done in igc.c via alloc_immovable in
>   igc_alloc_pseudovector. That allocated from from an AMS pool, which
>   doesn't use barriers.
>
> - It doesn't seem to access other Lisp objects except current_thread.
>
> That doesn't look bad, I think. Worth mentioning is perhaps that
> directly after get_backtrace here
>
>   static void
>   record_backtrace (struct profiler_log *plog, EMACS_INT count)
>   {
>     log_t *log = plog->log;
>     get_backtrace (log->trace, log->depth);
>     EMACS_UINT hash = trace_hash (log->trace, log->depth);
>
> we access Lisp objects in trace_hash when computing the hash and in the
> other hash table code. IIUC that code counts hits with the same
> backtrace. Don't know how long that takes. But if posting the backtrace
> would take the same time, we would be on par.
>
> I'll try to also look at SIGCHLD at some later point, but Christmas,
> family etc.
>
> Happy holidays!

Been up a bit early, so...

This is about SIGCHLD, and I must say I find it a bit hard to tell if
all other platforms do the same. There are simply too many #if's to
consider in the signal handling code.

Anyway, what I see here: SIGCHLD doesn't do anything dangerous in the
signal handler. Instead, the occurrence of SIGCHLD is added to a queue
with enqueue_async_work and that's basically it.

The work items in the queue are processed by process_pending_signals,
outside of the signal handler. Very nice, that's how it should be :-).
(And maybe, just as an inspiration, one could use that construct for
SIGPROF?)

So, there is actually no problem at all with SIGCHLD that I can see. 

My personal summary for SIGPROF + SIGCHLD at this point:

- I recommend rewriting SIGPROF handling in the way I tried to describe,
  possibly using the existing work queue mechanism. Everything else looks
  too complicated to me.
  
- Lisp allocation in signal handlers cannot exist because alloc.c is not
  reentrant which means we would crash with the old GC. We don't need
  anything extra for that in igc.

- No longer wondering why macOS does not show any problems in that whole
  area. The only problem is SIGPROF accessing Lisp objects, and the
  memory barrier is not a problem on macOS because it doesn't use
  signals.

Please double-check!