From fa3dd182094195d4b6d22c4a3db2eded8ff77c2b Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 18 Jan 2025 15:07:13 -0800 Subject: [PATCH 1/7] gh-129005: Avoid copy in _pyio.FileIO.readinto `os.read` allocated and filled a buffer by calling `read(2)`, than that data was copied into the user provied buffer. Read directly into the caller's buffer instead by using `os.readv`. `self.read()` was doing the closed and readable checks so move those into `readinto` --- Lib/_pyio.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 14961c39d3541d..ad1eca52e44fc5 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1695,10 +1695,12 @@ def readall(self): def readinto(self, b): """Same as RawIOBase.readinto().""" m = memoryview(b).cast('B') - data = self.read(len(m)) - n = len(data) - m[:n] = data - return n + self._checkClosed() + self._checkReadable() + try: + return os.readv(self._fd, (m, )) + except BlockingIOError: + return None def write(self, b): """Write bytes b to file, return number written. From b4c985a393ba76c20424d6e166198a495bfd38c0 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 18 Jan 2025 15:30:36 -0800 Subject: [PATCH 2/7] add blurb --- .../next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst diff --git a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst new file mode 100644 index 00000000000000..4d963686dcb5d1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst @@ -0,0 +1 @@ +Change ``_pyio.FileIO.readinto`` to use :func:`os.readv` avoiding a copy that was present when using :func:`os.read` From 78643f00fee823fab0a5978fe380a283661c6309 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 18 Jan 2025 15:52:12 -0800 Subject: [PATCH 3/7] Add conditional code for platforms without os.readv --- Lib/_pyio.py | 18 ++++++++++++------ ...5-01-18-15-29-08.gh-issue-129005.mM5dmV.rst | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index ad1eca52e44fc5..feffb0491c20ca 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1695,12 +1695,18 @@ def readall(self): def readinto(self, b): """Same as RawIOBase.readinto().""" m = memoryview(b).cast('B') - self._checkClosed() - self._checkReadable() - try: - return os.readv(self._fd, (m, )) - except BlockingIOError: - return None + if hasattr(os, 'readv'): + self._checkClosed() + self._checkReadable() + try: + return os.readv(self._fd, (m, )) + except BlockingIOError: + return None + else: + data = self.read(len(m)) + n = len(data) + m[:n] = data + return n def write(self, b): """Write bytes b to file, return number written. diff --git a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst index 4d963686dcb5d1..db3daf2b293e93 100644 --- a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst +++ b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst @@ -1 +1 @@ -Change ``_pyio.FileIO.readinto`` to use :func:`os.readv` avoiding a copy that was present when using :func:`os.read` +Change ``_pyio.FileIO.readinto`` to use :func:`os.readv` avoiding a copy that was present when using :func:`os.read` on platforms with :func:`os.readv` From c309705201b31a5b7a60e3d7d77e743f0b96e123 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 18 Jan 2025 15:53:41 -0800 Subject: [PATCH 4/7] tweak for early exit --- Lib/_pyio.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index feffb0491c20ca..ce13af5b34f0df 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1695,19 +1695,20 @@ def readall(self): def readinto(self, b): """Same as RawIOBase.readinto().""" m = memoryview(b).cast('B') - if hasattr(os, 'readv'): - self._checkClosed() - self._checkReadable() - try: - return os.readv(self._fd, (m, )) - except BlockingIOError: - return None - else: + if not hasattr(os, 'readv'): data = self.read(len(m)) n = len(data) m[:n] = data return n + self._checkClosed() + self._checkReadable() + try: + return os.readv(self._fd, (m, )) + except BlockingIOError: + return None + + def write(self, b): """Write bytes b to file, return number written. From 8907ca2233b01f06387d5b6e228db5b30dd2ca01 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sat, 18 Jan 2025 17:48:31 -0800 Subject: [PATCH 5/7] Inline self.read, simplifying overall --- Lib/_pyio.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index ce13af5b34f0df..6fea290df30d50 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1695,20 +1695,21 @@ def readall(self): def readinto(self, b): """Same as RawIOBase.readinto().""" m = memoryview(b).cast('B') - if not hasattr(os, 'readv'): - data = self.read(len(m)) - n = len(data) - m[:n] = data - return n - self._checkClosed() self._checkReadable() try: - return os.readv(self._fd, (m, )) + if hasattr(os, 'readv'): + return os.readv(self._fd, (m, )) + + data = os.read(self._fd, len(m)) + n = len(data) + m[:n] = data + return n except BlockingIOError: return None + def write(self, b): """Write bytes b to file, return number written. From a9ab9ee77a9038435fe4c02d9dc59a6626b424b5 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 19 Jan 2025 09:19:26 -0800 Subject: [PATCH 6/7] Update Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst Co-authored-by: Tomas R. --- .../next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst index db3daf2b293e93..314ba3e7fa0352 100644 --- a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst +++ b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst @@ -1 +1 @@ -Change ``_pyio.FileIO.readinto`` to use :func:`os.readv` avoiding a copy that was present when using :func:`os.read` on platforms with :func:`os.readv` +Optimize :meth:`_pyio.FileIO.readinto` by using :func:`os.readv` on platforms that support it which avoids an additional copy. From 9e3ef78cee65860d8eec9219e902fcacb63ed3c3 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 19 Jan 2025 09:34:45 -0800 Subject: [PATCH 7/7] Remove _pyio :meth: from NEWS as it results in a Sphinx warning --- .../next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst index 314ba3e7fa0352..fabb593ae58513 100644 --- a/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst +++ b/Misc/NEWS.d/next/Library/2025-01-18-15-29-08.gh-issue-129005.mM5dmV.rst @@ -1 +1 @@ -Optimize :meth:`_pyio.FileIO.readinto` by using :func:`os.readv` on platforms that support it which avoids an additional copy. +Optimize ``_pyio.FileIO.readinto`` by using :func:`os.readv` on platforms that support it which avoids an additional copy.