Use dynamic memoryview() size based on file size

As per #7160 (comment)
it appears using memoryview() on Windows is faster also for small files
if memoryview() length is equal to file size.

Author: giampaolo

Doc/library/shutil.rst

@@ -413,8 +413,8 @@ On Linux, Solaris and other POSIX platforms where :func:`os.sendfile` supports
 copies between 2 regular file descriptors :func:`os.sendfile` is used.
 
 On Windows :func:`shutil.copyfile` uses a bigger default buffer size (1 MiB
-instead of 16 KiB) and if file size >= 128 MiB a :func:`memoryview`-based
-variant of :func:`shutil.copyfileobj` is used.
+instead of 16 KiB) and a :func:`memoryview`-based variant of
+:func:`shutil.copyfileobj` is used.
 
 If the fast-copy operation fails and no data was written in the destination
 file then shutil will silently fallback on using less efficient

Doc/whatsnew/3.8.rst

@@ -100,9 +100,9 @@ Optimizations
   "fast-copy" means that the copying operation occurs within the kernel,
   avoiding the use of userspace buffers in Python as in
   "``outfd.write(infd.read())``".
-  On Windows :func:`shutil.copyfile` default buffer size was increased
-  from 16 KiB to 1 MiB and a faster implementation of :func:`shutil.copyfileobj`
-  is used for files bigger than 128 MiB.
+  On Windows :func:`shutil.copyfile` uses a bigger default buffer size (1 MiB
+  instead of 16 KiB) and a :func:`memoryview`-based variant of
+  :func:`shutil.copyfileobj` is used.
   The speedup for copying a 512 MiB file within the same partition is about
   +26% on Linux, +50% on macOS and +40% on Windows. Also, much less CPU cycles
   are consumed.

Lib/shutil.py

@@ -169,8 +169,11 @@ def _fastcopy_sendfile(fsrc, fdst):
                 break  # EOF
             offset += sent
 
-def _copybinfileobj(fsrc, fdst, length=COPY_BUFSIZE):
-    """readinto()/memoryview() based variant of copyfileobj()."""
+def _copyfileobj_readinto(fsrc, fdst, length=COPY_BUFSIZE):
+    """readinto()/memoryview() based variant of copyfileobj().
+    *fsrc* must support readinto() method and both files must be
+    open in binary mode.
+    """
     # Localize variable access to minimize overhead.
     fsrc_readinto = fsrc.readinto
     fdst_write = fdst.write
@@ -250,12 +253,10 @@ def copyfile(src, dst, *, follow_symlinks=True):
                     return dst
                 except _GiveupOnFastCopy:
                     pass
-            # Windows: for files >= 128 MiB in size we observe a
-            # considerable speedup by using a readinto()/memoryview()
-            # variant of copyfileobj(), see:
-            # https://github.com/python/cpython/pull/7160#discussion_r195162475
-            elif _WINDOWS and file_size >= 128 * 1024 * 1024:
-                _copybinfileobj(fsrc, fdst)
+            # Windows, see:
+            # https://github.com/python/cpython/pull/7160#discussion_r195405230
+            elif _WINDOWS and file_size > 0:
+                _copyfileobj_readinto(fsrc, fdst, min(file_size, COPY_BUFSIZE))
                 return dst
 
             copyfileobj(fsrc, fdst)

Lib/test/test_shutil.py

@@ -1936,35 +1936,34 @@ def test_file_offset(self):
             self.assertEqual(dst.tell(), self.FILESIZE)
 
     @unittest.skipIf(os.name != 'nt', "Windows only")
-    def test_alternate_win_impl(self):
-        # On Windows copyfile() uses copyfileobj() for files < 128 MiB,
-        # else an alternate memoryview()-based implementation.
-        def os_stat_mocked(path):
-            # Make shutil believe src file is 128 MiB.
-            if path == TESTFN:
-                mock = unittest.mock.Mock()
-                mock.st_size = 128 * 1024 * 1024
-                mock.st_mode = os.lstat(path).st_mode
-                return mock
-            else:
-                return os.lstat(path)
-
-        # Make sure it's not called.
-        with unittest.mock.patch("shutil._copybinfileobj") as m:
+    def test_win_impl(self):
+        # Make sure alternate Windows implementation is called.
+        with unittest.mock.patch("shutil._copyfileobj_readinto") as m:
             shutil.copyfile(TESTFN, TESTFN2)
-            assert not m.called
-        # Make sure it's called.
-        with unittest.mock.patch('shutil.os.stat', create=True,
-                                 side_effect=os_stat_mocked) as m1:
-            with unittest.mock.patch("shutil._copybinfileobj") as m2:
-                shutil.copyfile(TESTFN, TESTFN2)
-                assert m1.called
-                assert m2.called
-        # Test it.
-        with unittest.mock.patch('shutil.os.stat', create=True,
-                                 side_effect=os_stat_mocked) as m1:
-            shutil.copyfile(TESTFN, TESTFN2)
-        self.assert_files_eq(TESTFN, TESTFN2)
+        assert m.called
+
+        # File size is 2 MiB but max buf size should be 1 MiB.
+        self.assertEqual(m.call_args[0][2], 1 * 1024 * 1024)
+
+        # If file size < 1 MiB memoryview() length must be equal to
+        # the actual file size.
+        with tempfile.NamedTemporaryFile(delete=False) as f:
+            f.write(b'foo')
+        fname = f.name
+        self.addCleanup(support.unlink, fname)
+        with unittest.mock.patch("shutil._copyfileobj_readinto") as m:
+            shutil.copyfile(fname, TESTFN2)
+        self.assertEqual(m.call_args[0][2], 3)
+
+        # Empty files should not rely on readinto() variant.
+        with tempfile.NamedTemporaryFile(delete=False) as f:
+            pass
+        fname = f.name
+        self.addCleanup(support.unlink, fname)
+        with unittest.mock.patch("shutil._copyfileobj_readinto") as m:
+            shutil.copyfile(fname, TESTFN2)
+        assert not m.called
+        self.assert_files_eq(fname, TESTFN2)
 
 
 class _ZeroCopyFileTest(object):

Misc/NEWS.d/next/Library/2018-05-28-23-25-17.bpo-33671.GIdKKi.rst

@@ -3,8 +3,8 @@
 fast-copy syscalls on Linux, Solaris and macOS in order to copy the file
 more efficiently.
 On Windows :func:`shutil.copyfile` uses a bigger default buffer size (1 MiB
-instead of 16 KiB) and if file size >= 128 MiB a :func:`memoryview`-based
-variant of :func:`shutil.copyfileobj` is used.
+instead of 16 KiB) and a :func:`memoryview`-based variant of
+:func:`shutil.copyfileobj` is used.
 The speedup for copying a 512MiB file is about +26% on Linux, +50% on macOS and
 +40% on Windows. Also, much less CPU cycles are consumed.
 (Contributed by Giampaolo Rodola' in :issue:`25427`.)