On Monday, October 07, 2013 22:19:19 Daniel Stenberg wrote:
> On Mon, 7 Oct 2013, Kamil Dudka wrote:
> > ... if there is nothing to wait for. This fixes a regression introduced
> > by
> > commit 0feeab78 limiting the speed of SCP upload to 16384 B/s on a fast
> > connection (such as localhost).
>
> Thanks for the patch. I'm just a little concerned that this fix corrects the
> symptom rather than actually addresses the underlying problem.

Thanks for the reply. I hoped the patch would help to understand the issue.

> Let me explain what I suspect and once given some research we can decide on
> how to act on it.
>
> The change 0feeab78 you mention was a change that would prevent libcurl from
> busy-looping like crazy when it found nothing to wait on. (That code was
> later adjusted by d529f3882b.)
>
> The key here is the "nothing to wait on" part. When there's a transfer going
> on, even SCP, there should be a file descriptor to wait for[*] and the
> previously mentioned logic should not kick in. If that logic works, I don't
> think this patch is necessary.
>
> Since your patch clearly helps your case (as otherwise you wouldn't have
> posted it)

Indeed. The patch increased the SCP upload speed from ~16 KB/s to ~48 MB/s,
so we are definitely not dealing with a micro-optimization :-)

> I can only deduct that curl_multi_wait() in this case returns a
> zero in its fifth argument to signal that there's no file descriptor to wait
> for, and yet there should be as there's a transfer in progress!
>
> Did I understand this correctly?

Exactly!

> If so, we need to dig further why libcurl wrongly drops the file descriptor,
> right?

Then the problem seems to be in ssh_block2waitfor() as it explicitly clears
conn->waitfor unless it gets EAGAIN from the libssh2 layer. What is the
purpose of this logic? Perhaps a left-over from the legacy easy interface?

The logic pretends to work for SFTP, where each packet gets acknowledged by
server, which likely results in EAGAIN returned to the libcurl level in each
iteration. In case of SCP, libssh2 successfully writes the packet _without_
getting EAGAIN in each iteration. Consequently, conn->waitfor never gets a
non-zero value, curl_multi_wait() keeps returning zero file descriptors, and
the value without_fds in easy_transfer() keeps incrementing indefinitely.

> [*] = it will also return faster when there's a timeout that has expired but
> that shouldn't happen in multiple fast invokes so I don't think that's what
> you see happening.

Correct.

Kamil
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-10-08