GeezerVison is showing github.com — Reduce copies when reading files in pyio, match behavior of _io · Issue #129005 · python/cpython
Reduce copies when reading files in pyio, match behavior of _io #129005DescriptionFeature or enhancement
Proposal:
Currently _pyio uses ~2x as much memory to read all data from a file compared to _io. This is because it makes more than one copy of the data.
Details from test_fileio run
$ ./python -m test -M8g -uall test_largefile -m test_large_read -vvv
== CPython 3.14.0a4+ (heads/main-dirty:3829104ab41, Jan 17 2025, 21:40:47) [Clang 19.1.6 ]
== Linux-6.12.9-arch1-1-x86_64-with-glibc2.40 little-endian
== Python build: debug
== cwd: <$HOME>/python/build/build/test_python_worker_32392æ
== CPU count: 32
== encodings: locale=UTF-8 FS=utf-8
== resources: all
Using random seed: 1740056613
0:00:00 load avg: 0.53 Run 1 test sequentially in a single process
0:00:00 load avg: 0.53 [1/1] test_largefile
test_large_read (test.test_largefile.CLargeFileTest.test_large_read) ...
... expected peak memory use: 4.7G
... process data size: 2.3G
ok
test_large_read (test.test_largefile.PyLargeFileTest.test_large_read) ...
... expected peak memory use: 4.7G
... process data size: 2.3G
... process data size: 4.3G
... process data size: 4.7G
ok
----------------------------------------------------------------------
Ran 2 tests in 3.711s
OK
== Tests result: SUCCESS ==
1 test OK.
Total duration: 3.7 sec
Total tests: run=2 (filtered)
Total test files: run=1/1 (filtered)
Result: SUCCESS
Plan:
- Switch to
os.readv()os.readinto() to do readinto like C _Py_read used by _io does. os.read() can't take a buffer to use. This aligns behavior between _io.FileIO.readall and _pyio.FileIO.readall. os.readv works well today and takes a caller allocated buffer rather than needing to add a new os API. readv(2) mirrors the behavior and errors of read(2), so this should keep the same end behavior.
- Update
_pyio.BufferedIO to not force a copy of the buffer for readall when its internal buffer is empty. Currently it always slices its internal buffer then adds the result of _pyio.FileIO.readall to it.
For iterating, I'm using a small tracemalloc script to find where copies are:
from _pyio import open
import tracemalloc
with open("README.rst", 'rb') as file:
tracemalloc.start()
data = file.read()
snap = tracemalloc.take_snapshot()
stats = snap.statistics('lineno')
for stat in stats:
print(stat)Loose Ends
os.readv seems to be well supported but is currently guarded by a configure check. I'd like to just make pyio require readv, but can do conditional code if needed. If making readv non-optional generally is feasible, happy to work on that.
os.readv is not supported on WASI, so need to add conditional code.
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
No response
Linked PRs
Activitycmaloney commented on Jan 22, 2025 ContributorAuthor More actions I took a tangent and looked at the code complexity of adding os.readinto vs. the conditionals around os.readv not existing on some platforms + needing to pass a sequence of buffers to os.readv.
_pyio Using os.readv with conditional: https://github.com/python/cpython/pull/129006/files- Adding
os.readinto, _pyio using it: cmaloney@52c6013
Happy to work on either course, slight preference for os.readinto as it feels like by adding one function removes a lot of new conditional code to make these cases more efficient. Code wise, with the Buffer Protocol, the code to me gets quite a bit easier to follow. Not sure it's worth another public OS api to maintain though.
Know moving to a full PR for os.readinto would need to add tests and news, just focusing on "adding os.readinto" vs. "using os.readv".
cc: @tomasr8 , @vstinner , @gpshead (reviewers where os.read vs. _Py_read / readinto has come up). vstinner commented on Jan 22, 2025 Can you please open a separated issue for os.readinto()? This function looks useful and simple. cmaloney commented on Nov 18, 2025 ContributorAuthor More actions Reopening as _pyio.BufferedIO layer still can cause an unnecessary copy with its buf[pos:] + chunk; small patch for that shortly. cmaloney commented on Nov 19, 2025 ContributorAuthor More actions _pyio now has 5 allocations for reading in full which can be retrieved with the tracemalloc code in the main issue post:
# .take_bytes() retrieves the bytes from the `.readall()` bytearray
${WORKDIR}/cpython/Lib/_pyio.py:1741: size=8944 B, count=1, average=8944 B
# os.fstat allocates a stat result
${WORKDIR}/cpython/Lib/_pyio.py:1602: size=476 B, count=7, average=68 B
# Allocate the BufferedReader
${WORKDIR}/cpython/Lib/_pyio.py:258: size=328 B, count=1, average=328 B
# Allocate the FileIO
${WORKDIR}/cpython/Lib/_pyio.py:232: size=328 B, count=1, average=328 B
# Allocate the BufferedReader Lock
${WORKDIR}/cpython/Lib/_pyio.py:1055: size=56 B, count=1, average=56 Bthe b"" + b"data" case in BufferedReader.read is optimized in bytes_concat so does not copy. Could add a branch to avoid the bytes slicing + add call, but that doesn't significantly change the runtime in my testing so rather leave the code as is for now.
_pyio.BufferedReader is using a bytes as its buffer currently and I'd like to move that to a list of buffers (which could be bytes, memoryview, or bytearray) but that is out of scope for this issue. That's the current path I'm exploring in my more general buffered rework which I'm prototyping in _pyio.
morotti commented on Nov 19, 2025
_pyio.BufferedReader is using a bytes as its buffer currently
If it's of any help. I've experimented with that for the C buffering code from readall() which is bugged, it's reading the file in tiny chunks, increasing the buffer few KB at a time then 12.5% at a time after some threshold.
#131052
Using a list of bytes VS bytes made no improvement because the underlying C code is reallocating the buffer in place.
The reallocation is able to resize the buffer in place in microsecond. (I've only tested on Linux).
The hundred calls to gradually resize the buffer to readall a large file file actually takes no time, it doesn't copy any data. I was very surprised. cmaloney commented on Nov 19, 2025 ContributorAuthor More actions re: C readall(), #140139 should help a little :). A good general grow non-linearly readall loop is one of the things I've been trying to find... But so far the right easy to use and understand abstraction has been out of my grasp.
Here I was mainly focused on "_pyio is much slower than _io for some cases" and found these copies taking up a significant portion of the time in the largefile tests. Now within 15% which doesn't mean both can't be faster, just there's less of a delta. I'm using _pyio as a testing ground for other structural changes to io which should hopefully reduce I/O copies overall.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|