Repository navigation
ftplib and sendfile() #57773
Description
| BPO | 13564 |
|---|---|
| Nosy | @pitrou, @giampaolo, @merwok |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/giampaolo'
closed_at = None
created_at = <Date 2011-12-09.06:56:06.908>
labels = ['3.7', 'performance']
title = 'ftplib and sendfile()'
updated_at = <Date 2016-09-08.22:54:07.941>
user = 'https://github.com/giampaolo'bugs.python.org fields:
activity = <Date 2016-09-08.22:54:07.941>
actor = 'christian.heimes'
assignee = 'giampaolo.rodola'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2011-12-09.06:56:06.908>
creator = 'giampaolo.rodola'
dependencies = ['17552']
files = ['23890', '29336', '29351', '29357', '35568']
hgrepos = []
issue_num = 13564
keywords = ['patch']
message_count = 32.0
messages = ['149076', '149170', '149237', '183599', '183601', '183602', '183603', '183606', '183607', '183609', '183622', '183637', '183654', '183655', '183666', '183669', '183670', '183673', '183680', '183754', '183755', '183814', '183827', '183828', '183831', '183832', '183834', '183835', '184881', '184882', '185293', '220261']
nosy_count = 4.0
nosy_names = ['pitrou', 'giampaolo.rodola', 'eric.araujo', 'rosslagerwall']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue13564'
versions = ['Python 3.6', 'Python 3.7']
Activity
giampaolo commentedon Dec 9, 2011
In attachment.
This is actually just an excuse to store the patch somewhere and possibly collect opinions as I don't really think this should go in because:
merwok commentedon Dec 10, 2011
:)
pitrou commentedon Dec 11, 2011
os.fstat wouldn't work since it succeeds with non-"regular" files, e.g. standard I/O:
>>> os.fstat(0) posix.stat_result(st_mode=8592, st_ino=5, st_dev=11, st_nlink=1, st_uid=500, st_gid=5, st_size=0, st_atime=1323629303, st_mtime=1323629303, st_ctime=1323628616)I think the best solution is to call sendfile() and catch OSError, then fallback on the generic loop. However, you must also guard against fileno() failing:
neologix commentedon Mar 6, 2013
Here's the result of a benchmark sending a 1GB file over a Gb/s ethernet network:
vanilla:
real 0m9.446s
user 0m0.493s
sys 0m1.425s
sendfile:
real 0m9.143s
user 0m0.055s
sys 0m0.986s
The total time doesn't vary much (the reduction above is just jitter).
But it reduces user+sys time by almost a factor of 2.
Note that is changed Giampaolo's patch to call sendfile on the whole file, not by block.
giampaolo commentedon Mar 6, 2013
That's not compatible across POSIX platforms.
neologix commentedon Mar 6, 2013
What do you mean ?
I juste call fstat() before calling senfile() to find out the file
size, and pass it as
count.giampaolo commentedon Mar 6, 2013
Ah ok, I misinterpreted what you wrote then.
Generally speaking though, you don't need to know the file size: you just decide a blocksize (>= 65536 is usually ok) and use sendfile() as you use send().
neologix commentedon Mar 6, 2013
But then you make much more syscalls than what's necessary, and your
heuristic for the block size is never going to match the choice the
kernel makes.
Anyway, here are the numbers, do you think it's interesting to merge
(I mean, you're the Python ftp expert ;-) ?
giampaolo commentedon Mar 6, 2013
Specifying a big blocksize doesn't mean the transfer will be faster.
send/sendfile won't send more than a certain amount of bytes anyways.
If I'm not mistaken I recall from previous benchmarks that after a certain point (131072 or something) increasing the blocksize results in equal or even worse performances.
Another thing I don't like is that by doing so you implicitly assume that the file is "fstat-eable". I don't know if there are cases where it's not, but the less assumptions we do the better.
Note: I'm sure that for both send() and sendfile() blocksize>=65536 is faster than blocksize=8192 (the current default) so it probably makes sense to change that (I'll file a separate issue).
neologix commentedon Mar 6, 2013
The transfer won't be faster mainly because it's really I/O bound.
But it will use less CPU, only because you're making less syscalls.
I can perfectly believe this for a send loop, maybe because you're
exceeding the socket buffer size, or because your working set doesn't
fit into caches anymore, etc.
But for sendfile(), I don't see how calling it repeatedly could not be
slower than calling it once with the overall size: that's how netperf
and vsftpd use it, and probably others.
Well, the file must be mmap-able, so I doubt fstat() is the biggest concern...
giampaolo commentedon Mar 6, 2013
Have you actually measured this?
neologix commentedon Mar 7, 2013
"""
vanilla over Gb/s:
real 0m9.035s
user 0m0.523s
sys 0m1.412s
block-sendfile over Gb/s:
real 0m9.683s
user 0m0.253s
sys 0m1.212s
full-sendfile over Gb/s:
real 0m9.014s
user 0m0.059s
sys 0m1.000s
"""
As you can see, the throughput doesn't vary (the difference in "real
time" is just part of the variance).
However, the CPU usage (user+sys) is less for block-sendfile than send
loop, and less for full-sendfile than block-sendfile.
"""
vanilla over loopback:
real 0m3.200s
user 0m0.541s
sys 0m0.702s
block-sendfile over loopback:
real 0m2.713s
user 0m0.248s
sys 0m0.197s
full-sendfile over loopback:
real 0m1.718s
user 0m0.055s
sys 0m0.082s
"""
Same thing for loopback, except that here, zero-copy makes a
difference on the throughput because we're not I/O bound, but really
CPU/memory bound (and here sendfile of the complete file really
outperforms block-sendfile).
I don't have access to a 10Gb/s network, but basic math hints that
sendfile could make a difference on the overall throughput.
giampaolo commentedon Mar 7, 2013
It seems you're right, sorry. We need to take that into account then.
In the meantime I rewrote the original patch and got rid of the "use_sendfile" explicit argument in order to attempt to use sendfile() by default and fall back on using send() if bytes sent were 0.
TSL_FTP related changes are still left out for now as I'm planning a little refactoring first.
neologix commentedon Mar 7, 2013
"""
# block until socket is writable
select.select([], [sockno], [])
"""
I don't get it, why do you use select?
11 remaining items
pitrou commentedon Mar 9, 2013
EAGAIN on a blocking fd? Is it documented somewhere?
The Linux man page for sendfile() says:
write would block.
FreeBSD apparently says something similar:
giampaolo commentedon Mar 9, 2013
I see. Well, what I'm experiencing right now if I remove the select() / poll() call is a sequence of EAGAIN errors alternated by successful sendfile() calls.
Either the man page is wrong or I'm missing something.
pitrou commentedon Mar 9, 2013
Weird. I guess it would be nice to dig a bit more :-)
giampaolo commentedon Mar 21, 2013
After digging a bit further it seems EAGAIN occurs in case a timeout was previously set against the socket as in ftplib.FTP(..., timeout=2) (at least on Linux, FWICT).
As such, we can debate whether avoid using select/poll if timeout was not set.
I'll that a look at the man pages of the other POSIX platforms and figure whether EAGAIN is interpreted as on Linux.
Other than that, the patch is reasonably ok to me and can be committed as-is and blocksize argument tuning can be discussed in a separate ticket.
pitrou commentedon Mar 21, 2013
Ah, indeed. That's because socket timeout makes the underlying fd non-blocking.
Which means there probably should be a higher-level sendfile() facility for sockets, taking into account the socket timeout...
giampaolo commentedon Mar 26, 2013
I created bpo-17552 in order to discuss about socket.sendfile() addition.
giampaolo commentedon Jun 11, 2014
Updated patch which uses the newly added socket.sendfile() method (bpo-17552).