Add os.sendfile()

[rosslagerwall]

BPO	10882
Nosy	@loewis, @birkenfeld, @pitrou, @giampaolo, @skrah, @anacrolix
Files	sendfile.patch: Patch + test for issue sendfile_v2.patch: Updated to build on solaris sendfile_v2.patch: adds entries in pyconfig.h.in and configure and works on Linux sendfile_v3.patch: Updated for non-blocking sockets on certain platforms sendfile_v4.patch: use keyword args sendfile_v5.patch: updated documentation and tests sendfile_v6.patch: provides full test suite; fixes BSD issue with non-blocking sockets sendfile_v7.patch: skip headers/trailers test on linux and solaris; adds flags argument test sendfile_v8.patch: return bytes sent only instead of (bsent, offset)

^{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 = None
closed_at = <Date 2011-03-03.14:37:57.125>
created_at = <Date 2011-01-10.20:27:45.652>
labels = ['extension-modules', 'type-feature']
title = 'Add os.sendfile()'
updated_at = <Date 2011-09-02.02:14:55.132>
user = 'https://bugs.python.org/rosslagerwall'

bugs.python.org fields:

activity = <Date 2011-09-02.02:14:55.132>
actor = 'Yury.Selivanov'
assignee = 'none'
closed = True
closed_date = <Date 2011-03-03.14:37:57.125>
closer = 'giampaolo.rodola'
components = ['Extension Modules']
creation = <Date 2011-01-10.20:27:45.652>
creator = 'rosslagerwall'
dependencies = []
files = ['20338', '20352', '20357', '20573', '20578', '20597', '20688', '20708', '20733']
hgrepos = []
issue_num = 10882
keywords = ['patch']
message_count = 41.0
messages = ['125924', '125927', '125983', '125999', '126000', '126002', '126008', '126015', '126039', '126049', '126072', '126076', '126982', '126998', '127293', '127299', '127326', '127335', '127362', '127364', '127369', '127382', '127564', '128024', '128040', '128089', '128098', '128115', '128117', '128223', '128226', '128309', '129077', '129303', '129374', '129381', '129402', '129555', '129971', '136257', '136260']
nosy_count = 9.0
nosy_names = ['loewis', 'georg.brandl', 'pitrou', 'giampaolo.rodola', 'skrah', 'anacrolix', 'Yury.Selivanov', 'rosslagerwall', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue10882'
versions = ['Python 3.3']

[rosslagerwall]

Attached is a patch which implements os.sendfile for unix systems (linux, freebsd, apple, solaris, dragonfly).

It takes the iov initialization code and off_t parsing from i10812.

It encapsulates all the functionality from the various sendfiles which means a fair amount of #ifdefs but the basic case works for all of them.

Tested on Linux & FreeBSD - it should work on solaris but since it needs to link with the sendfile library and I have no idea how to link the posix module with the sendfile library only on Solaris, i couldn't test it. If someone could please contribute this...

I think it might be possible to get a Windows equivalent of this - i'll leave it for someone else to do ;-)

[pitrou]

Tested on Linux & FreeBSD - it should work on solaris but since it
needs to link with the sendfile library and I have no idea how to link
the posix module with the sendfile library only on Solaris, i couldn't
test it.

Since the posix module is linked statically inside the interpreter, it probably needs some change in the base ldflags. I'll take a look when I have some time.

[rosslagerwall]

Ok, I figured it out to link with sendfile on solaris. Here is the updated patch.

[giampaolo]

Thanks for writing this.
Follows my comments.

I would focus on trying to provide a unique interface across all platforms. Being sendfile() not a standard POSIX I think we should not worry about providing a strict one-to-one interface.

"headers" and "trailers" arguments should be available everywhere as they are crucial in different protocols such as HTTP.
On Linux this is possible by using the TCP_CORK option.
"man sendfile" on Linux provides some information. Also you might find useful to see how medusa did this (/medusa-20010416/sendfile/sendfilemodule.c):
http://www.nightmare.com/medusa/

The "offset" parameter should be available everywhere, Linux included (in your patch you dropped Linux support).
Also, I think it should be optional since when it's NULL, sendfile() implicitly assumes the current offset (file's tell() return value).
This is true on Linux, at least. Not sure about other platforms but my best guess is that it should not be mandatory.

It turns out the only peculiar argument is "flags", available only on *BSD.
I'm not sure what's best to do here. Maybe it makes sense to provide it across all platforms, defaulting to 0 and raise ValueError when specified on systems != *BSD.
In summary, the function might look like this:

sendfile(out, in, count, offset=None, headers=None, trailers=None, flags=0)

[giampaolo]

Also, I think it should be optional since when it's NULL, sendfile()
implicitly assumes the current offset (file's tell() return value).
This is true on Linux, at least. Not sure about other platforms but my
best guess is that it should not be mandatory.

I'm not so sure about this anymore. Please ignore this.

[rosslagerwall]

Just to be clear:

There are 3 different interfaces.
The basic one with the offset included & no headers/trailers is supported by all the platforms, including Linux.
The one with offset as None is only supported by Linux.
The one with headers/trailers/flags is supported by FreeBSD & OS X.

So it does provide a unique interface across all platforms while still providing the ability to access the native functionality.

Preferably, I'd like to see a thin wrapper like this remain and then have a sendfile() method added to the socket object which takes a file-like object (not a file descriptor) and optional headers/trailers. This method can then figure out how best to do it depending on the platform. (i.e. using TCP_CORK if necessary, etc). It could even be made to work with file-like objects that cannot be mmap()ed.

Why not put it straight in socket anyway? Well, some of the implementations allow sendfile() to have a normal fd as the output. Putting it in socket then would't make sense.

[giampaolo]

I agree then, although I'm not sure having a function with a variable number of args depending on the platform is acceptable.
I wanted to try your patch but it does not apply cleanly (python 3.2, revision 87930).

[rosslagerwall]

I've just tried it against r87935 and it applies cleanly.

Perhaps you didn't apply the patch correctly (it requires "-p1" since it was a Mercurial diff), try:
patch -p1 < sendfile_v2.patch

With regards to the different arguments, I don't know if that's acceptable or not or if there is a better way. Since you can have mmap.mmap() with differing args between Windows & Unix, maybe it is acceptable. And, Python exposes differing functionality via the posix module since the available functions differs widely between platforms.

[giampaolo]

The patch as-is did not work on Linux.
I had to add entries in pyconfig.h.in and configure files in order to make os.sendfile available.
Patch is in attachment.

[loewis]

I would focus on trying to provide a unique interface across all
platforms. Being sendfile() not a standard POSIX I think we should
not worry about providing a strict one-to-one interface.

We absolutely need to expose sendfile as-is. If we want to provide
some unifying layer, we must not call it sendfile.

[rosslagerwall]

Oh sorry, that was because it changed configure.in so "autoreconf" needs to be run to regenerate configure & pyconfig.h.in.

I thought that patches weren't meant to include the regenerated files. Especially since differences in the versions between autoconf 2.65 & 2.67 can produce hundreds of differences in "configure" making the patch large and obfuscating the actual changes.

[loewis]

I thought that patches weren't meant to include the regenerated files.

Correct. Not including them is perfectly fine.

[anacrolix]

I notice Linux is described as not taking count=0 to mean to send until the end of "in" is reached. Is it possible to pass (size_t)-1 to this field if None is given, or do a loop on non-zero counts from sendfile to emulate this?

I poked around the linux source for 2.6.32, and it appears sendfile() is emulated on top of splice(), but this doesn't provide undocumented count=0 handling as I was hoping.