bug#12327: Signal-handler cleanup for Emacs

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 02 Sep 2012 12:01:11 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12327@debbugs.gnu.org, lekktu@gmail.com
> 
> On 09/02/2012 10:51 AM, Eli Zaretskii wrote:
> 
> > that doesn't impede code reading and understanding in any way
> 
> Sure it does, because Emacs defines symbols incompatibly with their
> normal meanings.  E.g., Emacs redefines 'sigblock' to be a macro
> whose type signature is incompatible with the 'sigblock'
> that is found on GNU/Linux.

If that's the problem, then fix that, by changing the type.  Why do
more than is strictly needed?  Gratuitous changes are BAAAD!

> > I could go with replacing 'signal' etc. with their modern Posix
> > replacements, such as 'sigaction', directly in the code.
> 
> That would make the mainline code significantly less readable.
> Instead of this, for example:
> 
>   unblock_signal (SIGPIPE);
> 
> send_process_trap would have to do something like this:
> 
>   sigset_t mask;
>   ...
>   sigemptyset (&mask);
>   sigaddset (&mask, SIGPIPE);
>   pthread_sigmask (SIG_UNBLOCK, &mask, NULL);
> 
> which is not an improvement.

Of course, it's an improvement!  That kind of code is a pattern
programmers are used to see when signals are being masked.  By
contrast, when I see a call to unblock_signal, I need to look it up.

> I suspect the main reason that Emacs currently uses macros reminiscent
> but incompatible with the obsolete 4.2BSD interface, rather than the
> standard POSIX interface, is because the 4.2BSD interface was simpler.

No, it's just because the BSD interface was there first.

> Unfortunately, the 4.2BSD interface does not scale well to modern
> systems with lots of signals.

I suggested to use sigaction and friends, not go back to the old APIs.

> > I fail to see any good reasons for changes that, e.g., hide a
> > pair of calls to well-known library functions, such as 'sigemptyset'
> > and 'sigaddset', behind 'sigsetmask' (which AFAIK is a BSD-ism)
> 
> There must be some confusion here.

Yeah, did I say the new code is confusing?

> > As for 0.6% reduction in the size of .text: what kind of humongous
> > .text size do you have that makes 0.6% a significant value?
> 
> Every little bit helps, no?

Not every little bit can be justified by such non-trivial changes.
Changes come at a cost, which should be justified.

> But the main point of measuring text size
> is as a rough gauge of the efficiency implications.  If the text
> segment had grown, that would have been a bad sign.

Not necessarily.  And certainly not at fractions of a percent.

> Fewer instructions mean faster CPU performance, partly due to lower
> pressure on the instruction cache.

This all smells premature optimization to me.  Text segment size has
no direct relation to pressure on the instruction cache.  What matters
is the structure of the code in terms of branches and loops, and its
locality.  The size alone is almost meaningless.  I'm sure you know
that.

> I didn't bother to measure CPU performance earlier, since that takes
> more work, but I just now did that with an artificial benchmark that
> simply blocks and then unblocks SIGCHLD, and on my platform the
> proposed patch speeds up this benchmark by 15%.

A meaningless benchmark, IMO.  You need to measure an Emacs loop that
does these block and unblock operations (if there is such a loop), and
see the effect on that.  _That_ would be interesting.