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

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

[Moving discussion to cygwin-developers]

On Aug 29 11:57, Ken Brown via Cygwin wrote:
> On 8/29/2021 5:07 AM, Takashi Yano via Cygwin wrote:
> > On Sat, 28 Aug 2021 18:41:02 +0900
> > Takashi Yano wrote:
> > > On Sat, 28 Aug 2021 10:43:27 +0200
> > > Corinna Vinschen wrote:
> > > > On Aug 28 02:21, Takashi Yano via Cygwin wrote:
> > > > > On Fri, 27 Aug 2021 12:00:50 -0400
> > > > > Ken Brown wrote:
> > > > > > Two years ago I thought I needed nt_create to avoid problems when calling
> > > > > > set_pipe_non_blocking.  Are you saying that's not an issue?  Is
> > > > > > set_pipe_non_blocking unnecessary?  Is that the point of your modification to
> > > > > > raw_read?
> > > > > 
> > > > > Yes. Instead of making windows read function itself non-blocking,
> > > > > it is possible to check if the pipe can be read before read using
> > > > > PeekNamedPipe(). If the pipe cannot be read right now, EAGAIN is
> > > > > returned.
> > > > 
> > > > The problem is this:
> > > > 
> > > >    if (PeekNamedPipe())
> > > >      ReadFile(blocking);
> > > > 
> > > > is not atomic.  I. e., if PeekNamedPipe succeeds, nothing keeps another
> > > > thread from draining the pipe between the PeekNamedPipe and the ReadFile
> > > > call.  And as soon as ReadFile runs, it hangs indefinitely and we can't
> > > > stop it via a signal.
> > > 
> > > Hmm, you are right. Mutex guard seems to be necessary like pty code
> > > if we go this way.
> > 
> > I have found that set_pipe_non_blocking() succeeds for both read and
> > write pipes if the write pipe is created by CreateNamedPipe() and the
> > read pipe is created by CreateFile() contrary to the current create()
> > code. Therefore, not only nt_create() but also PeekNamedPipe() become
> > unnecessary.
> > 
> > Please see the revised patch attached.
> 
> That's a great idea.
> 
> I've applied your two patches to the topic/pipe branch.  I also rebased it
> and did a forced push in order to bring in Corinna's loader script fix.  So
> you'll have to do 'git fetch' and 'git rebase --hard origin/topic/pipe'.
> 
> Does this now fix all known problems with pipes?
> 
> Corinna, do you still see any benefit to switching to PIPE_NOWAIT?  AFAICT,
> it wouldn't decrease the code size at this point, so the only question is
> whether it might improve performance.

Pipes are already using PIPE_NOWAIT aka FILE_PIPE_COMPLETE_OPERATION
mode, see set_pipe_non_blocking.  The problem is that it's not used for
blocking pipes.  Rather, blocking pipes use overlapped IO.  Overlapped
IO is conceptually upside-down from the POSIX concept of non-blocking.
Also, the information returned in FilePipeLocalInformation is historically
borderline.  For kicks, see
https://cygwin.com/pipermail/cygwin-patches/2004q4/005002.html

So my suggestion is to try switching to non-blocking Windows pipes
entirely, even for blocking pipes on the user level.  It works nicely
for sockets.

> If you think it's worth trying, I'd be glad to code it up on a new branch,
> and we could compare the two.

We can do this in two version steps.  There's no pressure.

> Aside from that, I'm wondering how and when to merge the new pipe
> implementation to master.  It obviously needs much more widespread testing
> than it's gotten so far.  I'm a little nervous about it because I haven't
> thought about the details for two years, and no one other than me has tested
> it until a few days ago.

I could push out version 3.3.0, and afterwards you can just merge into
master.


Corinna