Thread: Optimising latch signals

Hi hackers,

Here are some more experimental patches to reduce system calls.

0001 skips sending signals when the recipient definitely isn't
waiting, using a new flag-and-memory-barrier dance.  This seems to
skip around 12% of the kill() calls for "make check", and probably
helps with some replication configurations that do a lot of
signalling.  Patch by Andres (with a small memory barrier adjustment
by me).

0002 gets rid of the latch self-pipe on Linux systems.

0003 does the same on *BSD/macOS systems.

The idea for 0002 and 0003 is to use a new dedicated signal just for
latch wakeups, and keep it blocked (Linux) or ignored (BSD), except
while waiting.  There may be other ways to achieve this without
bringing in a new signal, but it seemed important to leave SIGUSR1
unblocked for procsignals, and hard to figure out how to multiplex
with existing SIGUSR2 users, so for the first attempt at prototyping
this I arbitrarily chose SIGURG.

On Sun, Aug 9, 2020 at 11:48 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> Here are some more experimental patches to reduce system calls.
>
> 0001 skips sending signals when the recipient definitely isn't
> waiting, using a new flag-and-memory-barrier dance.  This seems to
> skip around 12% of the kill() calls for "make check", and probably
> helps with some replication configurations that do a lot of
> signalling.  Patch by Andres (with a small memory barrier adjustment
> by me).
>
> 0002 gets rid of the latch self-pipe on Linux systems.
>
> 0003 does the same on *BSD/macOS systems.

Here's a rebase over the recent signal handler/mask reorganisation.

Some thoughts, on looking at this again after a while:

1.  It's a bit clunky that pqinitmask() takes a new argument to say
whether SIGURG should be blocked; that's because the knowledge of
which latch implementation we're using is private to latch.c, and only
the epoll version needs to block it.  I wonder how to make that
tidier.
2.  It's a bit weird to have UnBlockSig (SIGURG remains blocked for
epoll builds) and UnBlockAllSig (SIGURG is also unblocked).  Maybe the
naming is confusing.
3.  Maybe it's strange to continue to use overloaded SIGUSR1 on
non-epoll, non-kqueue systems; perhaps we should use SIGURG
everywhere.
4.  As a nano-optimisation, SetLatch() on a latch the current process
owns might as well use raise(SIGURG) rather than kill().  This is
necessary to close races when SetLatch(MyLatch) runs in a signal
handler.  In other words, although this patch uses signal blocking to
close the race when other processes call SetLatch() and send us
SIGURG, there's still a race if, say, SIGINT is sent to the
checkpointer and it sets its own latch from its SIGINT handler
function; in the user context it may be in WaitEventSetWait() having
just seen latch->is_set == false, and now be about to enter
epoll_pwait()/kevent() after the signal handler returns, so we need to
give it a reason not to go to sleep.

By way of motivation for removing the self-pipe, and where possible
also the signal handler, here is a trace of the WAL writer handling
three requests to write data, on a FreeBSD system, with the patch:

kevent(9,0x0,0,{ SIGURG,... },1,{ 0.200000000 }) = 1 (0x1)
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0\0\M-/\^\"...,8192,0xaf0000) = 8192 (0x2000)
kevent(9,0x0,0,{ SIGURG,... },1,{ 0.200000000 }) = 1 (0x1)
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0 \M-/\^\\0"...,8192,0xaf2000) = 8192 (0x2000)
kevent(9,0x0,0,{ SIGURG,... },1,{ 0.200000000 }) = 1 (0x1)
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0`\M-/\^\\0"...,8192,0xaf6000) = 8192 (0x2000)

Here is the same thing on unpatched master:

kevent(11,0x0,0,0x801c195b0,1,{ 0.200000000 })   ERR#4 'Interrupted system call'
SIGNAL 30 (SIGUSR1) code=SI_USER pid=66575 uid=1001
sigprocmask(SIG_SETMASK,{ SIGUSR1 },0x0)         = 0 (0x0)
write(10,"\0",1)                                 = 1 (0x1)
sigreturn(0x7fffffffc880)                        EJUSTRETURN
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0`\M-w)\0\0"...,8192,0xf76000) = 8192 (0x2000)
kevent(11,0x0,0,{ 9,EVFILT_READ,0x0,0,0x1,0x801c19580 },1,{
0.200000000 }) = 1 (0x1)
read(9,"\0",16)                                  = 1 (0x1)
kevent(11,0x0,0,0x801c195b0,1,{ 0.200000000 })   ERR#4 'Interrupted system call'
SIGNAL 30 (SIGUSR1) code=SI_USER pid=66575 uid=1001
sigprocmask(SIG_SETMASK,{ SIGUSR1 },0x0)         = 0 (0x0)
write(10,"\0",1)                                 = 1 (0x1)
sigreturn(0x7fffffffc880)                        EJUSTRETURN
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0 \M-y)\0\0"...,8192,0xf92000) = 8192 (0x2000)
kevent(11,0x0,0,{ 9,EVFILT_READ,0x0,0,0x1,0x801c19580 },1,{
0.200000000 }) = 1 (0x1)
SIGNAL 30 (SIGUSR1) code=SI_USER pid=66575 uid=1001
sigprocmask(SIG_SETMASK,{ SIGUSR1 },0x0)         = 0 (0x0)
write(10,"\0",1)                                 = 1 (0x1)
sigreturn(0x7fffffffc880)                        EJUSTRETURN
read(9,"\0\0",16)                                = 2 (0x2)
kevent(11,0x0,0,0x801c195b0,1,{ 0.200000000 })   ERR#4 'Interrupted system call'
SIGNAL 30 (SIGUSR1) code=SI_USER pid=66575 uid=1001
sigprocmask(SIG_SETMASK,{ SIGUSR1 },0x0)         = 0 (0x0)
write(10,"\0",1)                                 = 1 (0x1)
sigreturn(0x7fffffffc880)                        EJUSTRETURN
pwrite(4,"\b\M-Q\^E\0\^A\0\0\0\0\0\M-z)\0"...,8192,0xfa0000) = 8192 (0x2000)
kevent(11,0x0,0,{ 9,EVFILT_READ,0x0,0,0x1,0x801c19580 },1,{
0.200000000 }) = 1 (0x1)
read(9,"\0",16)                                  = 1 (0x1)

The improvement isn't quite as good on Linux, because as far as I can
tell you still have to have an (empty) signal handler installed and it
runs (can we find a way to avoid that?), but you still get to skip all
the pipe manipulation.

On Fri, Nov 13, 2020 at 12:42 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> 1.  It's a bit clunky that pqinitmask() takes a new argument to say
> whether SIGURG should be blocked; that's because the knowledge of
> which latch implementation we're using is private to latch.c, and only
> the epoll version needs to block it.  I wonder how to make that
> tidier.

I found, I think, a better way: now InitializeLatchSupport() is in
charge of managing the signal handler and modifying the signal mask.

> 3.  Maybe it's strange to continue to use overloaded SIGUSR1 on
> non-epoll, non-kqueue systems; perhaps we should use SIGURG
> everywhere.

Fixed.

> The improvement isn't quite as good on Linux, because as far as I can
> tell you still have to have an (empty) signal handler installed and it
> runs (can we find a way to avoid that?), but you still get to skip all
> the pipe manipulation.

I received an off-list clue that we could use a signalfd, which I'd
discounted before because it still has to be drained; in fact the
overheads saved outweigh that so this seems like a better solution,
and I'm reliably informed that in a future WAIT_USE_IOURING mode it
should be possible to get rid of the read too, so it seems like a good
direction to go in.

I'll add this to the next commitfest.

On Thu, Nov 19, 2020 at 4:49 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> I'll add this to the next commitfest.

Let's see if this version fixes the Windows compile error and warning
reported by cfbot.