[issue25156] shutil.copyfile should internally use os.sendfile when possible

Change by Berker Peksag : -- superseder: -> Efficient zero-copy for shutil.copy* functions (Linux, OSX and Win) ___ Python tracker ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Giampaolo Rodola' added the comment: Closing as duplicate of #33671. -- resolution: -> duplicate stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

STINNER Victor added the comment: Different implementation: bpo-33639. -- nosy: +vstinner ___ Python tracker ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Change by Giampaolo Rodola' : -- nosy: +giampaolo.rodola ___ Python tracker ___ ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: If anyone is interested, I have created a package to monkey patch shutil.copyfile to benefit from sendfile, similarly to the last patch, but it also works on older Python versions down to 2.7. PyPI link: https://pypi.python.org/pypi/pyfastcopy/ --

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Can this patch be merged, or is there something I can do to improve it? -- ___ Python tracker ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Thank you SilentGhost for the second review on the v4 patch. Attached is the v5 patch which hopefully is getting even better. -- Added file: http://bugs.python.org/file41252/issue25156_v5.patch ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

SilentGhost added the comment: No further comments from me. I haven't run the test, but I trust it passes without any warnings. -- nosy: +SilentGhost ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Here is a new patch, with changes suggested by SilentGhost and josh.rosenberg in the review. -- Added file: http://bugs.python.org/file41243/issue25156_v4.patch ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Ping A small patch, but a good performance improvement :) -- ___ Python tracker ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Martin Panter added the comment: Also, man pages for Free BSD and OS X (where writing to a disk file is not supported) say it raises: * ENOTSUP if “the ‘fd’ argument does not refer to a regular file” * EBADF if “the ‘s’ argument is not a valid socket descriptor” * ENOTSOCK if “the ‘s’ argument

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Martin Panter added the comment: I left a few comments. But it might be good if someone more familiar with the APIs could review this. Have you seen the socket.sendfile() implementation? It’s a bit of a mess, and the circumstances are slightly different, but it may offer partial inspiration.

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Here is an updated patch that takes into account Martin's suggestions, both here and in the code review. -- Added file: http://bugs.python.org/file40911/issue25156_v3.patch ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

R. David Murray added the comment: I'm not at all sure this is worth the maintenance burden at this point in time. So let's say I'm -0.5. I agree that a review and opinion by someone more familiar with the API would be best. I'm adding gps as nosy since this feels like the kind of

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Here is an improved patch with the following changes: * Fallback to copyfileobj if sendfile fails with errno set to EINVAL or ENOSYS * Add a test for > 4GB file -- Added file: http://bugs.python.org/file40906/issue25156_v2.patch

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: I played a bit with Unix domain sockets, and it appears you can not open them like a file with open(). So they do no work with the current implementation of shutil.copyfile anyway. -- ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Thanks for the comment. > Also, the os.sendfile() doc suggests that some platforms only support writing > to sockets, so I definitely think a backup plan is needed. You are right, the man page clearly says: > Applications may wish to fall back to read(2)/write(2) in

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Martin Panter added the comment: Also, the os.sendfile() doc suggests that some platforms only support writing to sockets, so I definitely think a backup plan is needed. -- stage: -> patch review ___ Python tracker

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Thoughts anyone? Here is a patch that implements the change. My tests show a 30-40% performance improvement for 128KB-512MB single file copy: 128 KB file copy: $ dd if=/dev/urandom of=/tmp/f1 bs=1K count=128 Without the patch: $ ./python -m timeit -s 'import shutil;

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Josh Rosenberg added the comment: Adding interested parties from earlier ticket. -- nosy: +josh.r, martin.panter, r.david.murray ___ Python tracker ___

[issue25156] shutil.copyfile should internally use os.sendfile when possible

Martin Panter added the comment: I’ve never used sendfile() nor shutil.copyfile(), but my immediate reaction is maybe we need a backup plan if os.sendfile() is available but not supported in the circumstances. E.g. if it is practical to use copyfile() to copy from a named socket in the

[issue25156] shutil.copyfile should internally use os.sendfile when possible

desbma added the comment: Additional advantage of calling sendfile from shutil.copyfile: other fonctions in shutil module would automatically benefit from the use of senfile because they call copyfile directly (copy, copy2) or indirectly (copytree). So for example, the performance of

[issue25156] shutil.copyfile should internally use os.sendfile when possible

New submission from desbma: This is related to issue25063 (https://bugs.python.org/issue25063). Trying to use sendfile internally in shutil.copyfileobj was considered risky because of special Python files that expose a file descriptor but wrap it to add special behavior (eg: GzipFile). I