This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: very poor cygwin scp performance in some situations


Lev,

On Apr 26 21:18, Lev Bishop wrote:
> I've investigated the reports of poor network performance on cygwin
> [lots of interesting description erased]

thanks for doing this awful lot of debugging and the patch.  I see that
you didn't expect this patch getting applied without some discussion,
so, here we go.

> Corinna's patch should help when the pipe being select()ed on
> eventually terminates the select() and mine should help in the case
> that some other event terminates the select(), which is the case for
> scp (incoming data on the network socket terminates the select() ). I
> haven't tested this combination yet. I don't have sshd configured on
> my cygwin, so I haven't tested doing copies initiated from the openbsd
> side....
> 
> Anyway, with my patches to sockets and to thread_pipe(), I can scp
> from openbsd -> cygwin, initiated on cygwin, at 6.8MB/s, which is the
> same speed as in the other direction, and over 400% better than
> without the patch.

Just to clarify.  I looked into the performance problem, too, and it
turned out that the biggest problem in case of using scp was the 10ms
delay in thread_pipe.  The idea of my patch is to have practically no
delay in thread_pipe if a lot of data is going over the pipe.  In my
case, the performance gain was:

  cygwin-box> scp linux-box:file .
  ... 1.3 MB/s

vs.

  cygwin-box> scp linux-box:file .
  ... 11.8 MB/s

This *is* scp, so I'm not sure your change to thread_pipe is really
necessary anymore.  It adds another creation of an OS object on each
select invocation.  Hmm.

Btw., I also noticed the small 8K SO_SNDBUF and SO_RCVBUF sizes, so I
compared them with Linux.  As a result I changed net.cc, function
fdsock, to set the start values of the SO_SNDBUF and SO_RCVBUF buffers
to the same as on Linux, which is 85K rcv/16K snd for TCP connections,
resp. 120K for UDP or AF_LOCAL sockets (just local inet sockets, as
you're well aware, probably).  Your update_sndbuf function would have to
take this into account, though.

> So... my patches (against cygwin-1.5.19-4)  are attached.

That's a problem.  Please send patches against the latest from CVS.
That's where it's getting applied against and it's no fun to have to
manually tweak a patch to be able to apply it.

>  I decided not to introduce the overhead of
> synchronization structures, but maybe someone has comments?

Not yet.  I would also think sync'ing isn't of major importance here.

> To get maximum benefit, it's advised, but not required, to increase
> HKLM\System\CurrentControlSet\Services\Tcpip\Parameters\TcpWindowSize(DWORD)
> to an appropriate value for your network. 

We shouldn't think about this at all :-)  The registry settings aren't
really under our control, so we have to live with what we get.  However,
if it's useful to *know* the value, we could of course use the current
setting for further tweaking...

> I made an honest effort to format this patch correctly. I hope I did
> everything right.
> 
> Lev
> PS: If the maintainers want to incorporate this patch, and consider it
> "significant" enough to require a copyright assignment, I'll be happy
> to submit one.

A few comments:

The patch is slightly longer than the usual "10 lines are trivial" rule,
so, yes, you will have to sign and send the copyright assignment form.

The formatting of the patch seem to be broken as far as indentation
is concerned (by the mail client?)

In the ChangeLog we would prefer a new line for each changed or new
function.  Rather use "Ditto." than to put multiple function names into
one bracket expression.

I'd like to play with this patch and to apply it eventually, but it's a
bit late for 1.5.20, sorry.

This is my second last day until vacation and a 10 days hospital stay
right after that.  So I'm not available for the next four weeks.  Any
further action has to wait until June, sorry again.


Thanks for this patch and the really interesting discussion of the
internals.  It's highly appreciated.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]