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

[Takashi Yano]

Hi Corinna,

On Mon, 6 Sep 2021 10:13:10 +0200
Corinna Vinschen wrote:
> On Sep  6 05:09, Takashi Yano wrote:
> > On Sun, 5 Sep 2021 14:47:26 -0400
> > Ken Brown wrote:
> > > On 9/5/2021 9:50 AM, Takashi Yano wrote:
> > > > On Sun, 5 Sep 2021 22:40:59 +0900
> > > > Takashi Yano wrote:
> > > >> On Sun, 5 Sep 2021 08:15:23 +0900
> > > >> Takashi Yano wrote:
> > > >>> On Sat, 4 Sep 2021 10:04:12 -0400
> > > >>> Ken Brown wrote:
> > > >>>> On 9/4/2021 8:37 AM, Takashi Yano wrote:
> > > >>>>> On Sat, 4 Sep 2021 21:02:58 +0900
> > > >>>>> Takashi Yano wrote:
> > > >>>>>> On Fri, 3 Sep 2021 09:27:37 -0400
> > > >>>>>> Ken Brown wrote:
> > > >>>>>>> On 9/3/2021 8:22 AM, Takashi Yano wrote:
> > > >>>>>>>> POSIX says:
> > > >>>>>>>>        The value returned may be less than nbyte if the number of bytes left
> > > >>>>>>>>        in the file is less than nbyte, if the read() request was interrupted
> > > >>>>>>>>        by a signal, or if the file is a pipe or FIFO or special file and has
> > > >>>>>>>>                                                                          ~~~
> > > >>>>>>>>        fewer than nbyte bytes immediately available for reading.
> > > >>>>>>>>        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >>>>>>>>
> > > >>>>>>>> https://pubs.opengroup.org/onlinepubs/009604599/functions/read.html
> > > >>>>>>>>
> > > >>>>>>>> If it is turned over, read() should read all data immediately available,
> > > >>>>>>>> I think.
> > > >>>>>>> [...]
> > > >>>>>> Corinna Vinschen wrote:
> > > >>>>>>> We can't keep the mutex while waiting on the pending read, and there
> > > >>>>>>> could be more than one pending read running at the time.  if so,
> > > >>>>>>> chances are extremly high, that the data written to the buffer gets
> > > >>>>>>> split like this:
> > > >>>>>>> [...]
> > > >>>>> Takashi Yano wrote:
> > > >>>>>> Why can't we keep the mutex while waiting on the pending read?
> > > >>>>>> If we can keep the mutex, the issue above mentioned does not
> > > >>>>>> happen, right?
> > [...]
> > @@ -239,43 +237,51 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
> >        return;
> >      }
> >  
> > -  do
> > +  DWORD timeout = is_nonblocking () ? 0 : INFINITE;
> > +  if (WAIT_TIMEOUT == WaitForSingleObject (read_mtx, timeout))
> 
> My code was originally not supposed to serialise the readers.  The
> mutex block should be short lived and only create an atomic block
> for the two calls NtQueryInformationFile and NtReadFile.
> 
> If you have multiple readers, all but one of them will hang in this
> WFSO.  They will block here without a chance to kill or Ctrl-C them
> and thread cancellation won't work.
> 
> To fix that you have to use cygwait and handle signals and thread
> cancellation the same way as in the below code following the NtReadFile.

OK. Thanks for the advice. Then what about the patch attached?

> >  	  /* If the pipe is empty, don't request more bytes than half the
> > -	     pipe buffer size.  Every pending read lowers WriteQuotaAvailable
> > -	     on the write side and thus affects select's ability to return
> > -	     more or less reliable info whether a write succeeds or not.
> > -
> > -	     Let the size of the request depend on the number of readers
> > -	     at the time. */
> > +	     pipe buffer size. Pending read lowers WriteQuotaAvailable on
> > +	     the write side and thus affects select's ability to return
> > +	     more or less reliable info whether a write succeeds or not. */
> > +	  ULONG chunk = max_atomic_write / 2;
> >  	  status = NtQueryInformationFile (get_handle (), &io,
> >  					   &fpli, sizeof (fpli),
> >  					   FilePipeLocalInformation);
> > -	  if (NT_SUCCESS (status) && fpli.ReadDataAvailable == 0)
> 
> If the readers are serialized anyway, why fetch only half the remaining
> buffer size?  In that case fetching fpli.InboundQuota - 1 is as good
> as fetching just the half of it, isn't it?

It sounds reasonable. Adopted in the attached patch.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cygwin-pipe-Stop-counting-reader-and-read-all-availa.patch
Type: application/octet-stream
Size: 5370 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210906/daed3fa3/attachment.obj>