Repository navigation
Use high-performance os.sendfile() in shutil.copy* #77820
Description
| BPO | 33639 |
|---|---|
| Nosy | @facundobatista, @ncoghlan, @vstinner, @giampaolo, @bitdancer, @encukou, @vadmium, @desbma |
| PRs | |
| Superseder |
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 2018-05-28.16:28:30.230>
created_at = <Date 2018-05-24.20:15:29.809>
labels = ['3.8', 'library', 'performance']
title = 'Use high-performance os.sendfile() in shutil.copy*'
updated_at = <Date 2018-05-28.16:29:08.167>
user = 'https://github.com/giampaolo'bugs.python.org fields:
activity = <Date 2018-05-28.16:29:08.167>
actor = 'giampaolo.rodola'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-28.16:28:30.230>
closer = 'giampaolo.rodola'
components = ['Library (Lib)']
creation = <Date 2018-05-24.20:15:29.809>
creator = 'giampaolo.rodola'
dependencies = []
files = []
hgrepos = []
issue_num = 33639
keywords = ['patch']
message_count = 8.0
messages = ['317611', '317622', '317627', '317630', '317634', '317738', '317882', '317883']
nosy_count = 11.0
nosy_names = ['facundobatista', 'ncoghlan', 'vstinner', 'giampaolo.rodola', 'StyXman', 'r.david.murray', 'petr.viktorin', 'neologix', 'python-dev', 'martin.panter', 'desbma']
pr_nums = ['7100']
priority = 'normal'
resolution = 'duplicate'
stage = 'patch review'
status = 'closed'
superseder = '33671'
type = 'performance'
url = 'https://bugs.python.org/issue33639'
versions = ['Python 3.8']
Activity
giampaolo commentedon May 24, 2018
This is a follow up of bpo-25063 and similar to socket.sendfile() (bpo-17552). It provides a 20/25% speedup when copying files with shutil.copyfile(), shutil.copy() and shutil.copy2(). Differently from bpo-25063 this is used for filesystem files only and copyfileobj() is left alone.
Unmerged bpo-26826 is also related to this. I applied bpo-26826 patch and built a wrapper around copy_file_range() and the speedup is basically the same. Nevertheless, even when bpo-26826 gets merged it probably makes sense to rely on sendfile() in case copy_file_range() is not available (it was introduced in 2016) or the UNIX platform supports file-to-file copy via sendfile(2) (even though I'm not aware of any, so this would basically be Linux-only).
Some benchmarks:
$ dd if=/dev/urandom of=/tmp/f1 bs=1K count=128 $ time ./python -m timeit -s 'import shutil; p1 = "/tmp/f1"; p2 = "/tmp/f2"' 'shutil.copyfile(p1, p2)'128K copy
=========
--- without patch:
--- with patch:
8MB copy
========
--- without patch:
--- with patch:
512MB copy
==========
--- without patch:
--- with patch:
desbma commentedon May 24, 2018
Duplicate of https://bugs.python.org/issue25156 ?
giampaolo commentedon May 24, 2018
Oh! I got confused by the fact that bpo-25063 was rejected due to concerns about copyfileobj() otherwise I would have commented on your patch which I totally missed. Yes, this overlaps with bpo-25156 patch but it uses the same logic of socket.sendfile() which IMO is more robust. If you agree I'd close bpo-25156 and continue in here, and I'm more than happy to co-author the contribution if that's OK with you.
desbma commentedon May 24, 2018
Honestly, whatever gets this thing moving forward is good with me.
I'm a bit depressed by the number of issues here that have a good patch waiting to be merged, or even read, and that languish for years.
I haven't read your patch in detail, but if others agree that it is better than mine and can be merged, let's do it.
vstinner commentedon May 24, 2018
I'm really sorry about that. We, core developers, are doing our best, but we are 34 active core developers who merged 5000 pull requests in 1 year. We don't scale well :-(
We are working on getting more people onboard.
giampaolo commentedon May 26, 2018
I updated the code to rely on sendfile(2) on Linux only, which apparently is the only one supporting copy between regular files and added a check to fail immediately in case the filesystem is full.
Can somebody review the patch?
giampaolo commentedon May 28, 2018
Closing as duplicate of bpo-33671.
giampaolo commentedon May 28, 2018
Closing as duplicate of bpo-33671.