Expose new copy_file_range() syscall in os module.

[StyXman]

BPO	26826
Nosy	@facundobatista, @ncoghlan, @vstinner, @giampaolo, @encukou, @vadmium, @desbma, @pablogsal
PRs	bpo-26826: Expose copy_file_range in the os module #7255
Files	copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff copy_file_range.diff

^{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 2020-04-22.16:15:30.897>
created_at = <Date 2016-04-22.11:40:39.381>
labels = ['type-feature', 'library']
title = 'Expose new copy_file_range() syscall in os module.'
updated_at = <Date 2020-04-22.16:15:30.896>
user = 'https://bugs.python.org/StyXman'

bugs.python.org fields:

activity = <Date 2020-04-22.16:15:30.896>
actor = 'benjamin.peterson'
assignee = 'none'
closed = True
closed_date = <Date 2020-04-22.16:15:30.897>
closer = 'benjamin.peterson'
components = ['Library (Lib)']
creation = <Date 2016-04-22.11:40:39.381>
creator = 'StyXman'
dependencies = []
files = ['42619', '42628', '42638', '42640', '42651', '42708', '42829', '43310', '43318', '43319', '43624', '43639', '43640', '43688']
hgrepos = []
issue_num = 26826
keywords = ['patch']
message_count = 54.0
messages = ['264000', '264001', '264004', '264013', '264067', '264071', '264072', '264087', '264166', '264167', '264302', '264304', '264320', '264322', '264336', '264362', '264365', '264371', '264429', '264430', '264433', '264457', '264494', '264502', '264537', '264797', '265121', '265405', '265441', '267899', '268011', '268015', '268019', '269810', '269825', '269881', '269882', '270171', '274746', '317365', '317366', '317667', '317679', '337694', '344100', '344576', '344580', '344582', '344584', '344586', '344602', '344630', '344649', '344672']
nosy_count = 10.0
nosy_names = ['facundobatista', 'ncoghlan', 'vstinner', 'giampaolo.rodola', 'StyXman', 'petr.viktorin', 'neologix', 'martin.panter', 'desbma', 'pablogsal']
pr_nums = ['7255']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26826'
versions = ['Python 3.6']

[StyXman]

copy_file_range() has been introduced in the Linux kernel since version 4.5 (mid march 2016). This new syscall allows to copy data from one fd to another without passing by user space, improving speed in most cases. You can read more about it here:

https://lwn.net/Articles/659523/

I intend to start working on adding a binding for it in the os module and then, if it's available, use it in shutils.copy() to improve its efficiency. I have a couple of questions: If the syscall is not available, should I implement a user space alternative or should the method not exist at all?

[tiran]

Thanks, looks interesting.

We usually wait until syscalls are generally available in common distros and have bindings in glibc. It makes it easier to test the feature.

[vstinner]

We usually wait until syscalls are generally available in common distros and have bindings in glibc. It makes it easier to test the feature.

Usually, yeah. os.urandom() uses syscall() to use the new getrandom() of Linux since it's still not exposed in the GNU libc...
https://sourceware.org/bugzilla/show_bug.cgi?id=17252
Status: NEW

[vadmium]

Tangentially related: bpo-25156, about using sendfile() to copy files in shutil.

[StyXman]

Debian Sid, arch Linux and Fedora FC24 (alpha) already have linux-4.5, others would certainly follow soon. Meanwhile, I could start developing the patch and we could review it when you think it's appropriate.

[vstinner]

Kerbel support ok, but what about the libc support? Do you know if it is
planned? Or do you want to use the syscall() low-level API. If we take the
syscall() path, I suggest to make the function private in the os module.
Wait until the API is standardized in the libc. Sometimes the libc changes
minor things, ir's not always a thin wrapper to the syscall.

[StyXman]

Already there:

http://man7.org/linux/man-pages/man2/copy_file_range.2.html

[vstinner]

That's the manual page of the Linux kernel, not the glibc. It doesn't
mean that the glibc implemented it.

[StyXman]

Then I don't know. My only worry is whether having the method local to os would allow shutils.copy() to use it or not.

I will start by looking at other similar functions there to see to do it. urandom() looks like a good starting point.

[vstinner]

If we add a new private function to the os module (ex:
os._copy_file_range), it can be used in shutil if available. But it
would be a temporary solution until we declare the API stable. Maybe
it's ok to add the API as public today.

[StyXman]

Ok, I have a preliminary version of the patch. It has several parts:

• Adding the functionality to the os module, with docstring.
• Make shutil.copyfileobj() to use it if available.
• Modify the docs (this has to be done by hand, right?).
• Modify NEWS and ACKS.

Several points:

• For the time being, flags must be 0, so I was not sure whether put the argument or not. Just in case, I put it.

• I'm not sure how to test for availability, so configure defines HAVE_COPY_FILE_RANGE.

• No tests yet.

Talking about tests, I tried copying a 325MiB on an SSD, f2fs. Here are the times:

Old user space copy:

$ time ./python -m timeit -n 10 -s 'import shutil' 'a = open ("a.mp4", "rb"); b = open ("b.mp4", "wb+"); shutil.copyfileobj (a, b, 16*1024*1024)'
10 loops, best of 3: 259 msec per loop
real    0m7.915s
user    0m0.104s
sys     0m7.792s

New copy_file_range:

$ time ./python -m timeit -n 10 -s 'import shutil' 'a = open ("a.mp4", "rb"); b = open ("b.mp4", "wb+"); shutil.copyfileobj (a, b, 16*1024*1024)'
10 loops, best of 3: 193 msec per loop
real    0m5.926s
user    0m0.080s
sys     0m5.836s

Some 20% improvement, but notice that the buffer size is 1024 times Python's default size (16MiB vs. 16KiB).

One difference that I notice in semantics is that if the file is not open in binary form, but the file is binary, you get no UnicodeDecodeError (because the data never reaches userspace).

Let me know what you think.

[StyXman]

Version without the NEWS and ACKS change.

[StyXman]

Hmm, I just noticed that it doesn't fallback to normal copy if the arguments are not valid (EBADF, EXDEV). Back to the drawing board...