cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

[Takashi Yano]

On Wed, 8 Sep 2021 11:01:52 +0200
Corinna Vinschen wrote:
> On Sep  8 13:11, Takashi Yano wrote:
> > On Wed, 8 Sep 2021 09:07:48 +0900
> > Takashi Yano wrote:
> > > On Tue, 7 Sep 2021 19:50:23 +0900
> > > Takashi Yano wrote:
> > > 
> > > > @@ -796,7 +792,8 @@ pipe_cleanup (select_record *, select_stuff *stuff)
> > > >        pi->stop_thread = true;
> > > >        SetEvent (pi->bye);
> > >          ~~~~~~~~~~~~~~~~~~~
> > > This is not correct. SetEvent() wakes-up one of thread_pipe()s,
> > > but it may be other thread than one which should be stopped.
> > > 
> > > >        pi->thread->detach ();
> > > > -      CloseHandle (pi->bye);
> > > > +      if (me->fh->get_select_evt () == NULL)
> > > > +	CloseHandle (pi->bye);
> > > >      }
> > > >    delete pi;
> > > >    stuff->device_specific_pipe = NULL;
> > > 
> > > I think it also should be
> > > > +    for (ULONG i = 0; i < get_obj_handle_count (select_evt); i++)
> > > > +      SetEvent (select_evt);
> > > 
> > > Actually I want to use PulseEvent() here if it is not **UNRELIABLE**.
> > > https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/28648-pulseevent-is-an-unreliable-function
> > > 
> > > Does using semaphore object instead of event, and releasing
> > > resources equal to the number of handles make sense?
> > 
> > No it does not. One thread may consume semaphore multiple times....
> 
> What exactly is the problem in the code which results in high CPU
> load?  Can you explain this a bit?  Maybe we need an entirely
> different approach to avoid that.

The thread_pipe() code in the current git head of master is like:

while (looping)
  {
    ...
    if (peek_pipe ())
      looping = false;
    ...
    if (!looping)
      break;
    cygwait (pi->bye, sleep_time >> 3);
    if (sleep_time < 80)
      ++sleep_time;
    if (pi->stop_thread)
      break;
  }
returnn 0;

With this code, the first 8 loops calls cygwait() with
cygwait (pi->bye, 0);
and after that
cygwait (pi->bye, nonzero value);

cygwait() with nonzero timeout value causes usually sleeps at least
15msec (because of the resolution of the timer).

Looping 8 times is just a moment for the CPU.

After the 8 loops, thread_pipe() responds slowly to select() call
because thread notices the status change of pipe after the timeout.

To avoid this, commit dccde0dc changes the code as follows.

diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 8ad982c12..83e1c00e0 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -735,6 +735,7 @@ thread_pipe (void *arg)
   select_pipe_info *pi = (select_pipe_info *) arg;
   DWORD sleep_time = 0;
   bool looping = true;
+  DWORD t0 = GetTickCount ();

   while (looping)
     {
@@ -754,7 +755,12 @@ thread_pipe (void *arg)
        break;
       cygwait (pi->bye, sleep_time >> 3);
       if (sleep_time < 80)
-       ++sleep_time;
+       {
+         DWORD t1 = GetTickCount ();
+         if (t0 != t1)
+           ++sleep_time;
+         t0 = t1;
+       }
       if (pi->stop_thread)
        break;
     }

In this code, I expected t0 != t1 happens at most every 1msec,
and after 8msec, timeout for cygwait() becomes nonzero. During
this 8msec, CPU gets high load because looping is without sleep.

However, in fact, t0 != t1 happens every 15msec because the
resolution of timer is low. Then, CPU gets high load during
first 120msec after starting thread_pipe(). So if you type
keys quickly, the loop is always performs without sleep.

Therefore, I proposed new patch which use event or semaphore
to notify read()/write()/close() of the pipe instead of just
sleeping a while.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>