eduardok / libsmbclient-php

smbclient's extension for PHP
Other
99 stars 21 forks source link

Stream wrapper slower than the direct API #81

Open georgschoelly opened 3 years ago

georgschoelly commented 3 years ago

Overview

The stream wrappers are really useful. However, we noticed that copy operations are much slower. The client runs on Linux and the server on Windows.

For a 65 MB file we took the following measurements:

Tool Time
smbclient (CLI) 2.3s
stream copy() 47s
stream fwrite() 47s
smbclient_write 3s

Code

stream copy

copy("/path/to/file", "smb://server/C/testfile");

stream fwrite

$f_in = fopen("/path/to/file", "r");
$f_out = fopen("smb://server/C/testfile", "w");

while (!feof($f_in))
       fwrite($f_out, fread($f_in, 10*1024*1024));

smbclient_write

$state = smbclient_state_new();
smbclient_state_init($state);

$file = smbclient_creat($state, 'smb://server/C/testfile');
$f_in = fopen("/path/to/file", "r");

while (!feof($f_in)) {
        $data = fread($f_in, 10*1024*1024);
        smbclient_write($state, $file, $data);
}
smbclient_close($state, $file);
smbclient_state_free($state);
georgschoelly commented 3 years ago

A tcpdump of the SMB connection shows that the "fast" code sends many more packets between each response of the server. This suggests that the stream wrapper waits for a response from the server before sending more data.

georgschoelly commented 3 years ago

We also noticed that readfile("smb://server/file) is slower than using the "native" smbclient_read function.

remicollet commented 3 years ago

Sorry, cannot reproduce (on linux, using a Samba server)

Output with progress info:

Using API
10485760 en 0.03
20971520 en 0.06
31457280 en 0.09
41943040 en 0.12
52428800 en 0.15
62914560 en 0.17
73400320 en 0.20
79421010 en 0.22
Using Stream
10485760 en 0.03
20971520 en 0.06
31457280 en 0.09
41943040 en 0.12
52428800 en 0.15
62914560 en 0.17
73400320 en 0.20
79421010 en 0.22
remicollet commented 3 years ago

I can indeed see slower process using stream, but only with PHP 7, same speed with PHP 8

Using API
79421010 en 0.22

Using Stream
79421010 en 0.68
remicollet commented 3 years ago

After digging, stream API with PHP 7 split write in 8K bloc, so run much more calls

IMHO, nothing we can do in this extension side.

remicollet commented 3 years ago

For memory (chunk_size usage dropped in 8)

PHP 7: https://github.com/php/php-src/blob/PHP-7.4/main/streams/streams.c#L1122 PHP 8: https://github.com/php/php-src/blob/PHP-8.0/main/streams/streams.c#L1120

remicollet commented 3 years ago

This can be workaround in userland with

stream_set_chunk_size($f_out, 10*1024*1024);

@eduardok can also be managed in extension using

diff --git a/smb_streams.c b/smb_streams.c
index f70c490..e2f8aff 100644
--- a/smb_streams.c
+++ b/smb_streams.c
@@ -323,6 +323,7 @@ php_stream_smb_opener(
        smbc_open_fn smbc_open;
        SMBCFILE *handle;
        php_smb_stream_data *self;
+       php_stream *stream;

        /* Context */
        state = php_smb_pool_get(context, path TSRMLS_CC);
@@ -351,7 +352,9 @@ php_stream_smb_opener(
        self->state = state;
        self->handle = handle;

-       return php_stream_alloc(&php_stream_smbio_ops, self, NULL, mode);
+       stream = php_stream_alloc(&php_stream_smbio_ops, self, NULL, mode);
+       stream->chunk_size = 1*1024*1024*1024;
+       return stream;
 }

 static int

But I don't know if this is really a good idea to differ from the default value, especially as I don't see any extension using such change, so probably better to keep this for userland (perhaps some words in docs)

georgschoelly commented 3 years ago

Thank you for the excellent research and explanation. The commit that removed the chunking is https://github.com/php/php-src/commit/5cbe5a538c92d7d515b0270625e2f705a1c02b18. There's even a PHP bug for the slow writes: https://bugs.php.net/bug.php?id=73262

Setting the chunk size also fixes the slow read speed. Here's my test script:

$f_in = fopen("smb://server/testfile", "r");
$f_out = fopen("/dev/null", "w");

stream_set_chunk_size($f_in, 10 * 1024 * 1024);  // <-- helps a lot
stream_set_chunk_size($f_out, 10 * 1024 * 1024); // <-- helps just a little, i.e. negligible

stream_copy_to_stream($f_in, $f_out);

Regarding the proposed changes:

stream = php_stream_alloc(&php_stream_smbio_ops, self, NULL, mode);
stream->chunk_size = 1*1024*1024*1024;
return stream;

I saw that the HTTP wrapper changes the chunk size temporarily, so this might be ok to do in code: https://github.com/php/php-src/blob/3e01f5afb1b52fe26a956190296de0192eedeec1/ext/standard/http_fopen_wrapper.c#L351

/* avoid buffering issues while reading header */
if (options & STREAM_WILL_CAST)
    chunk_size = php_stream_set_chunk_size(stream, 1);
georgschoelly commented 3 years ago

I just tested PHP 8 now. PHP 8 fixes the write performance of the stream wrappers. The read performance is still bad though and requires the workaround with stream_set_chunk_size().

stamepicmorg commented 3 years ago

hello! i am using this module with nextclod-21 server for connection my external dfs-mountpoint (smb). (servers are located on neighboring virtual machines in the same 10-gigabit LAN).

my stable config is debian10 + php7.4 + v1.0.0.6 and i have very slooooooow and degrated performance with it on connected smb-drive. i also tested php7.3 and php8.0 + various veersions of this lib (1.0.0.0 to 1.0.0.6) from pecl.

a also had reinstalled nextcloud from scratch on various pure test systems - and i have same bad result. I noticed that performance was degraded since january 2021.. when i downloading files - i have very slow download speed. about 1-3Mb per sec, although earlier it was maximum, since I am on the same local network.

maybe i should downgrade to old version of nextcloud (that supports php7.2) and install it on 18.04 ubuntu server that includes old stable version of this php-module (0.8.0)..

sorry for my bad english, i am a little jaded of researching and debugging this problem.

best regards.