After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773632 - smb: Remove maximum read size during reads
smb: Remove maximum read size during reads
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: smb backend
unspecified
Other All
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2016-10-28 12:01 UTC by Bastien Nocera
Modified: 2016-11-04 19:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
smb: Remove maximum read size during reads (1.33 KB, patch)
2016-10-28 12:01 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2016-10-28 12:01:30 UTC
.
Comment 1 Bastien Nocera 2016-10-28 12:01:34 UTC
Created attachment 338719 [details] [review]
smb: Remove maximum read size during reads

The smb read call can take any arbitrary size and the both the
libsmbclient SMB1 and SMB2 engines will break this down into as
many simultaneous on the wire calls as needed to pipeline
the reads / writes.

Restricting this to 65534 will be slow, so remove this
restriction.

See https://lists.samba.org/archive/samba/2016-October/204225.html
Comment 2 Bastien Nocera 2016-10-28 12:38:03 UTC
With gvfs-1.30.1.1-1.fc25.x86_64:

$ gvfs-copy -p smb://diskstation.local/bastien/ISOs/ubuntu-14.04-desktop-amd64.iso  .
Transferred 1.0 GB out of 1.0 GB (21.5 MB/s)


With gvfs master and this patch applied:

$ gvfs-copy -p smb://diskstation.local/bastien/ISOs/ubuntu-14.04-desktop-amd64.iso  .
Transferred 1.0 GB out of 1.0 GB (21.1 MB/s)

This is over Wi-Fi, so the speed might be hitting the limits of the medium. The difference in speed is probably within error limits too. I added some debug, and something is clamping the buffer sizes:
** Message: reading 65536
That's 0x10000. Couldn't find references to either constants in glib/gio or gvfs though.
Comment 3 Ondrej Holy 2016-11-01 14:37:26 UTC
Review of attachment 338719 [details] [review]:

It is not really clear, why the limit was added in smb backend, but it probably remains just for "compatibility" reasons:
https://bugzilla.gnome.org/show_bug.cgi?id=588391#c13

Hmm, I am ok to remove it if samba upstream claims it is wrong...
Comment 4 Ondrej Holy 2016-11-01 14:38:02 UTC
Hmm, the size is limited by the copy_stream_with_progress's buffer[1024*64]:
https://git.gnome.org/browse/glib/tree/gio/gfile.c#n2768

It is worth to mention that GVfsReadChannel has 128 * 1024 limit:
https://git.gnome.org/browse/gvfs/tree/daemon/gvfsreadchannel.c#n107
Comment 5 Bastien Nocera 2016-11-02 10:59:16 UTC
Bumping the buffer in copy_stream_with_progress() to 128k raises the throughput slightly, to about 23 megs per second.

I tried bumping it to 256k seems to provide another speed bump, to around 28 megs a second average on my best run.
Comment 6 Bastien Nocera 2016-11-02 11:00:26 UTC
Attachment 338719 [details] pushed as de97010 - smb: Remove maximum read size during reads

Also filed https://bugzilla.gnome.org/show_bug.cgi?id=773823 against gio
Comment 7 Ondrej Holy 2016-11-02 11:22:07 UTC
(In reply to Bastien Nocera from comment #5)
> Bumping the buffer in copy_stream_with_progress() to 128k raises the
> throughput slightly, to about 23 megs per second.
> 
> I tried bumping it to 256k seems to provide another speed bump, to around 28
> megs a second average on my best run.

Hmm, isn't this limited by the GVfsReadChannel limit in this case? Maybe we should also enlarge it a bit...
Comment 8 Bastien Nocera 2016-11-02 11:53:45 UTC
(In reply to Ondrej Holy from comment #7)
> (In reply to Bastien Nocera from comment #5)
> > Bumping the buffer in copy_stream_with_progress() to 128k raises the
> > throughput slightly, to about 23 megs per second.
> > 
> > I tried bumping it to 256k seems to provide another speed bump, to around 28
> > megs a second average on my best run.
> 
> Hmm, isn't this limited by the GVfsReadChannel limit in this case? Maybe we
> should also enlarge it a bit...

Indeed, and that does boost the speed further, 29.65 MB/s over 10 separate runs of 1GB downloads with a maximum of 31.6 MB/s.

I filed https://bugzilla.gnome.org/show_bug.cgi?id=773826
Comment 9 Jeremy Allison 2016-11-04 19:13:44 UTC
Thanks for getting to this before I did Bastien.

Regards,

Jeremy.