Can you undo change in sig_dispatch_pending() ?

[Oleg Volkov <oleg at unicorn dot kiev dot ua>]

Hi, Cygwin Developers!

My problem is in that our company can't upgrade Cygwin from version
1.5.19-4 to current (1.5.24-2), because an application used by our
company begins to work several times slower. This performance degradation
is not specific to that application; other applications, which perform
input/output intensively (e.g. databases), can begin (and had already
begun) to work slower.

We had found out that critical change between versions of cygwin1.dll,
which degrades performance, was made on 2006-02-24 in the function
sig_dispatch_pending() in file `sigproc.c'. This function is called
indirectly from two critical functions, read() and write(), and from
other less important functions.

In this function the following has been changed:

  /* Force the wait_sig thread to wake up and scan for pending signals */
  void __stdcall
  sig_dispatch_pending (bool fast)
  {
    // if (exit_state || &_my_tls == _sig_tls || !sigq.start.next)  // version 1.5.19-4
       if (exit_state || &_my_tls == _sig_tls)                      // version 1.5.24-2
      {
        // ...
        return;
      }

    // ...
    sig_send (myself, fast ? __SIGFLUSHFAST : __SIGFLUSH);
  }

To see how this change affects performance, you can compile test program,
and run it with current version of cygwin1.dll (1.5.24-2), and with
current version, in which function sig_dispatch_pending() reverted back to
the state it was in version 1.5.19-4. Test program is given below.

  #include <fcntl.h>
  #include <unistd.h>

  int main(int argc, char **argv)
  {
      char chunk[64]="";
      int i, fd;
      if ((fd=open("tst_chunks.bin",
                   O_CREAT|O_WRONLY|O_TRUNC,
                   0666))<0) return 1;
      for (i=0; i<1000000; i++)
          if (write(fd,chunk,sizeof(chunk))!=sizeof(chunk)) return 1;
      close(fd);
      return 0;
  }

When invoked using `time -p <test_program>' command, it executes on my
Celeron 1.3GHz with cygwin1.dll version

  * 1.5.24-2                                    - 48 seconds;
  * 1.5.19-4 or 1.5.24-2 with the change undone - 18 seconds.

My question is: can you revert this change back, so read() and write() will
be as fast, as they were in version 1.5.19-4?.. Reverting the change back
seems not to affect Cygwin stability. My arguments are given below.

As it goes from the name of the function sig_dispatch_pending(), this
function forcedly dispatches signals, which were collected so far and need
to be sent to working application, if it has registered appropriate signal
handlers, or processed by default handlers, or ignored. Normally, signals
are continuously dispatched by special "signal" thread; immediate forcible
dispatching is performed by sig_dispatch_pending() function. Purpose to
invoke this function from read(), write() and other functions, which
contain possibly long-lasting Win32 calls, is to dispatch all collected
signals before system will be blocked during these calls, so signals which
were not dispatched in time by "signal" thread will not be dispatched much
later, after Win32 call finishes.

New signals, which need to be dispatched, are added to the end of queue of
pending signals, implemented as linked list, which is stored in variable
`sigq'. A pointer to the first pending signal can be obtained by expression
`sigq.start.next'. As we can see, in version 1.5.19-4 there was a check:
if queue of pending signals is empty, then return immediately; otherwise
function sig_send() with special argument is called, by means of which
time-consuming immediate signal dispatching is triggered. Why this check
was removed?

There are lines in ChangeLog:

2006-02-24  Christopher Faylor  <cgf at timesys dot com>

      * sigproc.cc (sigheld): Define new variable.
>       (sig_dispatch_pending): Don't check sigq since that's racy.
        (sig_send): Set sigheld flag if __SIGHOLD is specified, reset it if
        __SIGNOHOLD is specified.  Ignore flush signals if we're holding
        signals.

So it was considered that check of `sigq.start.next' introduces racing
condition, and this affects Cygwin stability. But this suggestion is wrong.

If the point is that two threads will refer to `sigq.start.next' pointer
simultaneosly, when, say, "signal" thread has updated first two bytes of
the pointer, and other thread begins to check condition `!sigq.start.next',
then this is not a problem, because this can never occur. Reading and
updating a pointer is performed atomically by CPU, since pointers are of
size `int', and reads and assignments to `int' are atomic. And even if it
is not performed atomically, then only two outcomes are possible: when
`sigq.start.next' is zero, and when it is non-zero. If these outcomes are
mixed in rare cases, then this causes no harm, as I will show below.

Let's examine situation for function sig_dispatch_pending() in version
1.5.19-4, when condition `sigq.start.next' was true at the time of the
check, but all signals were dispatched by "signal" thread in background.
This will result in idle call to sig_send(), which will execute normally,
since in version 1.5.24-2 sig_send() is called in any case, even if
there are no pending signals, and everything works well (but slowly).

Let's examine situation, when condition `sigq.start.next' was false, that
is, there were no pending signals at the time of the check, and there will
be immediate return from the function, so some signals, which were
collected a little later, will not be dispatched immediately, as if they
could be dispatched in version 1.5.24-2. In any case, those rare
latercoming signals will be dispatched by "signal" thread, not immediately,
just like other signals, which can occur during execution of read() or
write() after sig_dispatch_pending() was called; indeed, there are many
lines of code between call to sig_dispatch_pending() and Win32 call, which
actually performs input/output, so the situation, when some rare signals
are left undispatched before Win32 call, can not be prevented completely.

I hope I have convinced you that sig_dispatch_pending() function can be
reverted back to the state it was in version 1.5.19-4. Doing so will
increase Cygwin performance and will not affect Cygwin stability.


Oleg Volkov