Page MenuHomePhabricator

libssh sftp client is using unreasonable amount of memory
Closed, ResolvedPublic

Description

The qemu is using libssh2 at this moment. Switching to libssh is blocked on performance issues. Simple test transferring 128 MB allocates almost 10x times more data and roughly around 2x more memory than libssh2.

libssh:
==23832== HEAP SUMMARY:
==23832==     in use at exit: 127,621 bytes in 854 blocks
==23832==   total heap usage: 389,345 allocs, 388,491 frees, 1,368,567,364 bytes allocated

libssh2:

==23432== HEAP SUMMARY:
==23432==     in use at exit: 127,957 bytes in 856 blocks
==23432==   total heap usage: 152,729 allocs, 151,873 frees, 550,742,657 bytes allocated

Event Timeline

Jakuje created this task.Aug 28 2018, 5:14 PM
asn claimed this task.Aug 28 2018, 5:15 PM
asn triaged this task as Normal priority.Sep 4 2018, 8:13 PM

There are some small improvement with 574f279f00922e0cb732f61b752e8f6ba4c4dca5

asn added a comment.Sep 10 2018, 1:34 PM

I've did some more work on sftp. It would be great if you could review it.

https://gitlab.com/cryptomilk/libssh-mirror/commits/master-buffer

Here are some numbers:

master:
torture_sftp_benchmark ...........   Passed   15.33 sec
torture_sftp_benchmark ...........   Passed   15.54 sec
torture_sftp_benchmark ...........   Passed   14.97 sec


==15138== HEAP SUMMARY:
==15138==     in use at exit: 200 bytes in 4 blocks
==15138==   total heap usage: 1,804,712 allocs, 1,804,708 frees, 3,111,125,853 bytes allocated


master-buffer:
torture_sftp_benchmark ...........   Passed   14.48 sec
torture_sftp_benchmark ...........   Passed   15.12 sec
torture_sftp_benchmark ...........   Passed   15.30 sec


==17320== HEAP SUMMARY:
==17320==     in use at exit: 224 bytes in 5 blocks
==17320==   total heap usage: 1,689,963 allocs, 1,689,958 frees, 2,570,055,196 bytes allocated

The performance gain isn't much. So memory allocations don't really seem to be a huge problem. However to further improve the situation with memory allocations we need:

ssh_channel_writev()
ssh_channel_readv()

Like writev(2) and readv(2).

I added my review to gitlab.

Testing with qemu (128 MB) shows times around 7 seconds, which is now comparable to libssh2. Memory usage from valgrind says the following:

+==19477== HEAP SUMMARY:
+==19477==     in use at exit: 127,645 bytes in 855 blocks
+==19477==   total heap usage: 315,560 allocs, 314,705 frees, 695,361,676 bytes allocated

which is huge difference from the original version I was testing with. I am not sure why your valgrind says significantly more than my testing with qemu. Could it be because you trace also the sshd server?

asn added a comment.Sep 17 2018, 9:24 AM

I didn't run the qemu io test, I wrote a benchmark test for sftp which writes and reads 256MB. That's where the numbers above are from. I've changed the test to 128MB now.

The above valgrind report was for direct comparison with the data from original report in description. In that case, we got to 50 % of memory usage since we started, which is a good progress and what was my point. The patches looked good to me as already discussed.

I assume the largest difference was the moving of the data from packet layer to sftp layer, which is now passing the whole buffer.

asn closed this task as Resolved.Sep 20 2018, 12:34 PM

This has been pushed to master and will be released with libssh 0.8.3