classification
Title: socket.sendfile()
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, christian.heimes, giampaolo.rodola, neologix, pitrou, rosslagerwall
Priority: normal Keywords: needs review, patch

Created on 2013-03-26 19:27 by giampaolo.rodola, last changed 2014-04-20 18:57 by giampaolo.rodola.

Files
File name Uploaded Description Edit
socket-sendfile.patch giampaolo.rodola, 2013-03-26 19:27 initial draft review
socket-sendfile2.patch giampaolo.rodola, 2013-04-08 15:21 update file offset, add 'offset' parameter review
socket-sendfile3.patch giampaolo.rodola, 2014-04-20 18:56 docs, selectors module, always seek() file, use memoryview(), ssl tests review
Messages (6)
msg185292 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2013-03-26 19:27
This is based on progress made in issue 13564 and aims to provide a new sendfile() method for the socket class taking advantage of high-performance "zero-copy" os.sendfile() available on most POSIX platforms.
A wrapper on top of os.sendfile() appears to be convenient because getting everything right is a bit tricky. Also we can avoid code duplication in case we want to add sendfile() support to ftplib (issue 13564) and httplib.

=== THE API ===

Attached is a draft patch proposing an API which:

- uses os.sendfile() whenever available and usable (a mmap-like file is required)
- if not falls back on using plain socket.sendall()
- returns a tuple of two elements which indicates whether sendfile() was used and an exception instance in case it wasn't on account of an internal error, if any 
- does not support non-blocking sockets (will raise ValueError)


=== ALTERNATE API === 

1) In case the transfer is interrupted halfway because of an error the user has no clue on how many bytes were sent (and file.tell() won't tell =)). As such it probably makes sense to provide a custom socket.SendfileError exception encapsulating the original exception and the bytes sent as arguments.

2) For the same reason the returned tuple should probably include the number of bytes transmitted.

=== WINDOWS SUPPORT ===

Further development may include Windows support by using TransmitFile (http://msdn.microsoft.com/en-us/library/windows/desktop/ms740565(v=vs.85).aspx). Once we agree on an API I might start looking into Windows code (which appears to be quite tricky btw).

Any feedback is very welcome.
msg185294 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-03-26 19:43
A couple of comments:

- I don't understand the point of the second member in the tuple
- the timeout logic should be fixed so that the total operation time doesn't exceed the timeout, rather than each iteration (in other words, a deadline should be computed at the start and then the poll / select calls be issued in accordance with that deadline)
- non-blocking sockets could be supported by returning partial writes, and letting the caller pass an offset argument
- as a vocabulary nit, I don't really understand what "mmap-like file" means
msg185295 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2013-03-26 19:53
- I don't understand the "running out of FDs" thing. select() is limited to FDs less than FD_SETSIZE, but I don't see how you could get EMFILE (select() will return a ValueError)

- is there any platform with sendfile() which doesn't support poll()?

- I still don't see the point of calling sendfile() with a block size. Everybody just uses st_size, and that's how it's supposed to be done.
msg185303 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2013-03-26 21:05
> I don't understand the point of the second member in the tuple

The 'exception' member can be useful to know the reason why sendfile() failed and send() was used as fallback.


> the timeout logic should be fixed so that the total operation
> time doesn't exceed the timeout, rather than each iteration

Do you mean that if the user sets a timeout=2 and the whole transfer takes longer than that you expect a timeout exception?
That's pretty unusual (e.g. ftplib does not behave like that).


> non-blocking sockets could be supported by returning partial writes,
> and letting the caller pass an offset argument

I see little value in supporting non-blocking sockets because someone willing to do that usually wants to use sendfile() directly (no wrappers involved) as they are forced to handle errors and take count of transmitted bytes in their own code anyway.
Here's an example of how sendfile is supposed to be used with non-blocking sockets:
#1035">https://code.google.com/p/pyftpdlib/source/browse/tags/release-0.7.0/pyftpdlib/ftpserver.py#1035
An extra layer such as the one proposed in my patch is completely useless and also slow considering the amount of instructions before the actual sendfile() call.
It is however a good idea to provide an 'offset' argument.


> as a vocabulary nit, I don't really understand what "mmap-like file" means

Will turn that into "regular files".


> I don't see how you could get EMFILE (select() will return a ValueError)

Right. Will fix that.

> is there any platform with sendfile() which doesn't support poll()?

No idea. I made a quick tour on snakebite and apparently all of the provided platforms have poll(). 


> I still don't see the point of calling sendfile() with a block size

I will make some actual benchmarks and fix that later if makes sense.
Assuming providing a high blocksize (say 1MB) makes no difference in terms of CPU usage it's better to keep that for consistency with sendall() in case we fallback on using it.
msg186305 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2013-04-08 15:14
New patch in attachment includes a new 'offset' parameter, new tests and also update file offset on return or in case of error so that file.tell() can be used to tell how many bytes were transmitted at any time.
This way we'll avoid using a custom exception.

In summary, the API looks like this.

Transfer ok:

>>> file = open('somefile', 'rb')
>>> s = socket.socket()
>>> sock.sendfile(file)
(True, None)
>>> file.tell()
20000000
>>>


...and in case sendfile() could not be used internally because file was not a regular file:

>>> file = io.BytesIO(b'x' * 1*1024*1024)
>>> sock.sendfile(file)
(False, UnsupportedOperation('fileno',))
>>> file.tell()
20000000
>>>


I still haven't looked into TransmitFile on Windows as I have to figure out how to compile Python 3.4 on Windows.
msg216915 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-04-20 18:56
New patch in attachment. Changes:

- docs

- replaced select() / poll() with the new selectors module

- file position is always updated both on return and on error; this means file.tell() is the designated way to know how many bytes were sent

- replaced sendall() with send() so that we can count the number of bytes transmitted (related and rejected proposal: https://mail.python.org/pipermail/python-ideas/2014-April/027689.html)

- send() now uses memoryview() for better performances to re-transmit data which was not sent by the first send() call

- pre-emptively raise exception if file is not opened in binary mode

- tests for ssl module

I've tried to work on Windows TransmitFile support but I got stuck as I'm not sure how to convert a file object into a HANDLE in C. I suppose Windows support can also be added later as a separate ticket and in the meantime I'd like to push this forward.

Open questions: 

- Is the current return value desirable (do we really care if os.sendfile() was used internally?)? Should the returned tuple also include the number transmitted bytes?

- default blocksize: Charles-Fran├žois was suggesting to remove the blocksize argument; FWIW I've made some quick benchmarks by using "time" cmdline utility with different blocksizes and I didn't notice substantial difference. I still think a blocksize parameter is necessary in case we fallback on using send() and also for consistency with ftplib's storbinary() method which will be involved later (issue 13564).
History
Date User Action Args
2014-04-20 18:57:38giampaolo.rodolasetversions: + Python 3.5, - Python 3.4
2014-04-20 18:57:00giampaolo.rodolasetfiles: + socket-sendfile3.patch

messages: + msg216915
2013-04-08 15:21:38giampaolo.rodolasetfiles: + socket-sendfile2.patch
2013-04-08 15:21:23giampaolo.rodolasetfiles: - socket-sendfile2.patch
2013-04-08 15:15:13giampaolo.rodolasetfiles: + socket-sendfile2.patch
2013-04-08 15:14:11giampaolo.rodolasetmessages: + msg186305
2013-04-08 14:57:11giampaolo.rodolasettitle: create_server -> socket.sendfile()
2013-04-08 14:56:45giampaolo.rodolasettitle: socket.sendfile() -> create_server
2013-04-07 13:31:33asvetlovsetnosy: + asvetlov
2013-03-27 07:42:04rosslagerwallsetnosy: + rosslagerwall
2013-03-26 21:05:31giampaolo.rodolasetmessages: + msg185303
2013-03-26 20:13:47christian.heimessetnosy: + christian.heimes
2013-03-26 19:53:29neologixsetmessages: + msg185295
2013-03-26 19:43:24pitrousetnosy: + pitrou, neologix
messages: + msg185294
2013-03-26 19:39:46giampaolo.rodolalinkissue13564 dependencies
2013-03-26 19:27:53giampaolo.rodolacreate