Improve SFTP performance on medium/high latency connections

[zybexXL]

This PR changes just 2 constants:

• Increases maxPendingReads from 10 to 100
• Increases socket send/receive buffer to 10 SSH packets (320K)

This results in huge improvements in transfer speeds when using sftpClient.DownloadFile/UploadFile, in particular with fast local internet connections and remote servers with high latency. The results are even better when used together with PR #865 - speeds are now comparable with Filezilla.

I tested on two servers: a relatively close one (Frankfurt) and a distant one (AWS-hosted SFTP Server). Here are the results:

Server   Ping  Pending  PR865  Iterations               Average
------  -----  -------  -----  -------------------      ----------
FRA      30ms     10      No   29.59  28.90  26.55  =>  28.35 MB/s
FRA      30ms    100      No   43.28  37.74  41.54  =>  40.85 MB/s (144%)
FRA      30ms    100     YES   65.38  67.76  66.97  =>  66.70 MB/s (235%) [link limit 500Mbps]
  
AWS     100ms     10      No    7.01   6.69   7.45  =>   7.05 MB/s 
AWS     100ms    100      No   19.97  21.31  20.96  =>  20.74 MB/s (294%)
AWS     100ms    100     YES   25.50  26.36  26.22  =>  26.02 MB/s (369%)

The 500Mbps link is satisfyingly saturated when combining these 2 PRs! 😎

Reasoning

1. Increasing the socket send/receive buffer size
This change has low impact. This buffer holds packets that have been received and are waiting to be processed by the application. In cases where the machine is temporarily bogged down, it may happen that the incoming packets are not processed quickly enough and the previous 2-packet buffer gets full, and a 3rd incoming packet then causes the connection to stall. Increasing the buffer to 10 packets reduces the probability of this scenario, and makes the transfer more smooth and resilient to hiccups in CPU usage.

2. Increasing the maxPendingReads
This setting controls the maximum number of SFTP Read Requests that are sent to the server and are yet unanswered. When a packet arrives at the client, it sends another request for the next packet. In other words, it's the maximum number of in-flight data packets.

Each SFTP data packet is 32KB in size, so maxPendingReads=10 means that there's a maximum of 320 KB in flight before the transfer stalls. With 100, this is raised to 3.2MB in flight.

Modern internet connections are FAST. Transferring 320KB at 100Mbps takes about 30ms, and at 500Mbps it takes just 6.4ms (plus latency, ignoring details). This means that when talking to a server that is 100ms away this happens:

• SSH.Net sends 10 read requests
• connection is idle for 100ms, while the requests and replies are on their way
• ~100ms later, the 10 packets arrive: this takes about 10ms on a fast connection
• as the 10 packets arrive, SSH.Net sends 10 more requests
• the connection is now idle for ~90ms, waiting for the 2nd batch
• the loop repeats - the connection is idle up to 90% of the time!

The numbers are not accurate and I'm ignoring lots of detail but you get the idea. 320KB of in-flight data is just not suitable for today's fast connections anymore. Raising it to ~3.2MB fixes this problem (for now), and restores performance on distant servers.

[darkoperator]

Avery nice performance boost! This has been a complaint for a long time

…

On Aug 30, 2021, at 3:41 PM, zybexXL ***@***.***> wrote: @zybexXL commented on this pull request. In src/Renci.SshNet/ServiceFactory.cs <#866 (comment)>: > @@ -132,7 +132,7 @@ public ISftpFileReader CreateSftpFileReader(string fileName, ISftpSession sftpSe { var fileAttributes = sftpSession.EndLStat(statAsyncResult); fileSize = fileAttributes.Size; - maxPendingReads = Math.Min(10, (int) Math.Ceiling((double) fileAttributes.Size / chunkSize) + 1); + maxPendingReads = Math.Min(100, (int)Math.Ceiling((double)fileAttributes.Size / chunkSize) + 1); this line is also changed on PR #864 <#864> - here <https://github.com/sshnet/SSH.NET/pull/864/files#diff-abfc42015c3bccaae3e2012df18a1b2a24d66c6effb9ef5e30cbe6516023f967R135> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#866 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD7IHSQNLHAOK4ITGGLEWDT7PNHPANCNFSM5DCO2DGA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[zybexXL]

note: This value needs to be the same as ServiceFactory.cs:115

[zybexXL]

note: This value needs to be the same as ServiceFactory.cs:145
I've renamed this testcase to match what it does

[drieseng]

@zybexXL Could you check how many read-aheads putty performs? I recall that it does not perform that many, but I could be wrong. How does it compare with SSH.NET on speed/throughput ?

[zybexXL]

@drieseng, according to Wireshark, psftp filezilla sends 128 requests in about 6ms. Data starts arriving 30ms later from my remote test server, and then psftp sends a new requests every ~32KB of received data.

EDIT: I'm running tests with psftp now - it seems it only sends a couple of read-aheads, and it's much slower than Filezilla. I'll post results soon.

[zybexXL]

Confirmed, psftp sends only a single read request and waits for the data to arrive before sending another one. There's always a gap of a few ms between each 32KB block; this is not that bad when the server is close, but for distant servers and fast connections it just kills the speed. It ends up at about half-speed compared to this PR.

For SSH.NET with/without this patch, see benchmarks on top post. Filezilla, with the 128 read-aheads, has identical speed to SSH.NET with #865 and #866 applied.

Ideally the number of read-aheads should be dynamic to adjust to distance+speed automatically. That could be explored in a future PR, perhaps needed when 10Gbit speeds become common.

[IgorMilavec]

Just for future reference, here is how the dotnet team tackled a similar issue in HTTP/2: https://devblogs.microsoft.com/dotnet/dotnet-6-networking-improvements/

[mmttim]

Any updates on merging this one to the main branch? We are using SSH.NET for one of our tools in our build pipeline and we can clearly see a speed difference between FileZilla and SSH.NET.

We could switch to this branch off course, but it's more pleasent to just include the NuGet package.

[jjxtra]

Code doesn't compile for me, complaining about target frameworks being empty

[zybexXL]

I just pulled the branch and compiled in VS2019 with no problem. This code does not touch any Framework/Project config file.
There are compile warnings about net.core 2.1/2.2/3.0 being deprecated, but that comes from the main line and can be ignored for now.

[A9G-Data-Droid]

I have tested this and it works!

[geoffmca]

@drieseng Are we able to get this reviewed and added for a future release? Are there pending issues/concerns that can be addressed?

[zybexXL]

I've rebased this PR, please review and consider merging.
I have been using this code since 2021 and I still see the huge performance gains this provides.

[WojciechNagorski]

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1