[issue13564] ftplib and sendfile()

Changes by Christian Heimes : -- versions: +Python 3.6, Python 3.7 -Python 3.5 ___ Python tracker ___

[issue13564] ftplib and sendfile()

Changes by Giampaolo Rodola' g.rod...@gmail.com: Added file: http://bugs.python.org/file35568/ftplib-sendfile5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13564 ___

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: Updated patch which uses the newly added socket.sendfile() method (issue 17552). -- assignee: - giampaolo.rodola type: enhancement - performance versions: +Python 3.5 -Python 3.4 ___ Python tracker

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: I created issue 17552 in order to discuss about socket.sendfile() addition. -- dependencies: +socket.sendfile() ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13564

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: 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). Ah, indeed. That's because socket timeout makes the underlying fd non-blocking. Which

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: I don't understand why you must put the socket in non-blocking mode for sendfile(). Also I think the support code (_use_send() / _use_sendfile()) should be factored out somewhere. There could even be a socket.sendfile() method with the appropriate fallbacks?

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: I don't understand why you must put the socket in non-blocking mode for sendfile(). I did that mainly because I'm using select() / poll() and it seems kind of natural to set the socket in non-blocking mode (proftpd does the same). I'm not sure whether it

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: I don't understand why you must put the socket in non-blocking mode for sendfile(). I did that mainly because I'm using select() / poll() and it seems kind of natural to set the socket in non-blocking mode (proftpd does the same). But why do you need

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: Because otherwise sendfile() fails with EAGAIN many times before sending any actual data. select() / poll() make sure the while loop awakens only when the socket is ready to be written (as opposed to continuously catching EAGAIN and wait for sendfile() to

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: Because otherwise sendfile() fails with EAGAIN many times before sending any actual data. EAGAIN on a blocking fd? Is it documented somewhere? The Linux man page for sendfile() says: EAGAIN Nonblocking I/O has been selected using O_NONBLOCK and the

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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. --

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: 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. Weird. I guess it would be nice to dig a

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: Because offsets can be negative On Linux (and presumably on all POSIX platforms) passing a negative offset results in EINVAL. In that case, there's a problem with the patch, since select can block arbitrarily long because it doesn't take the socket

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: A much larger patch which should address all issues is in attachment. Updates: - use poll() instead of select() whenever possible - take socket timeout into account - take SSL/FTPS into account - when using select() look for EMFILE in case num fds

[issue13564] ftplib and sendfile()

Changes by Giampaolo Rodola' g.rod...@gmail.com: -- versions: +Python 3.4 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13564 ___ ___

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: 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. Have you actually measured this? vanilla over Gb/s: real0m9.035s user0m0.523s sys 0m1.412s

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: 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. # block until socket is writable select.select([],

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: It's necessary because sendfile() can fail with EAGAIN. As for your blocksize = filesize argument I changed my opinion: despite being less CPU consuming we might incur into problems if that number is too big. 'count' parameter on Linux, for example, is

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: It's necessary because sendfile() can fail with EAGAIN. It can fail with EAGAIN if the input FD is non-blocking, exactly like the current implementation which calls fp.read(). Furthermore, since sendfile actually supports only regular file and regular

[issue13564] ftplib and sendfile()

Antoine Pitrou added the comment: As for your blocksize = filesize argument I changed my opinion: despite being less CPU consuming we might incur into problems if that number is too big. 'count' parameter on Linux, for example, is expected to be an unsigned int. Other plarforms will also

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 'count' is size_t, like for mmap() and any other function accepting a length, so nothing wrong can happen. Then why 'offset' and 'count' parameters have a different data type? Linux: sendfile(..., off_t *offset, size_t count); Solaris: sendfile(...,

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: Then why 'offset' and 'count' parameters have a different data type? Because offsets can be negative (e.g. for lseek), while a size can't. That's why 'count' is size_t, not ssize_t. Furthermore, since sendfile actually supports only regular file and

[issue13564] ftplib and sendfile()

Changes by Charles-François Natali cf.nat...@gmail.com: -- nosy: -neologix ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13564 ___ ___

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: Here's the result of a benchmark sending a 1GB file over a Gb/s ethernet network: vanilla: real0m9.446s user0m0.493s sys 0m1.425s sendfile: real0m9.143s user0m0.055s sys 0m0.986s The total time doesn't vary much (the reduction

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: Note that is changed Giampaolo's patch to call sendfile on the whole file, not by block. That's not compatible across POSIX platforms. -- ___ Python tracker rep...@bugs.python.org

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: That's not compatible across POSIX platforms. What do you mean ? I juste call fstat() before calling senfile() to find out the file size, and pass it as `count`. -- ___ Python tracker

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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(). -- ___

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: 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(). But then you make much more syscalls than

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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

[issue13564] ftplib and sendfile()

Charles-François Natali added the comment: 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. The transfer won't be faster mainly because it's really I/O bound. But it will use less CPU, only because

[issue13564] ftplib and sendfile()

Giampaolo Rodola' added the comment: 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. Have you actually measured this? -- ___ Python tracker

[issue13564] ftplib and sendfile()

Antoine Pitrou pit...@free.fr added the comment: 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,

[issue13564] ftplib and sendfile()

Changes by Ross Lagerwall rosslagerw...@gmail.com: -- nosy: +rosslagerwall ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue13564 ___ ___

[issue13564] ftplib and sendfile()

Éric Araujo mer...@netwok.org added the comment: deciding whether using sendfile() should probably be done silently (no explicit argument) As an optimization taking advantage from OS support, I think this should be automatic too. But if there are too many issues, then explicit argument

[issue13564] ftplib and sendfile()

New submission from Giampaolo Rodola' g.rod...@gmail.com: 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: - it's UNIX only - as such, deciding whether using sendfile() should probably