GeezerVison is showing github.com — Speed up open().read() pattern by reducing the number of system calls · Issue #120754 · python/cpython
Speed up open().read() pattern by reducing the number of system calls #120754DescriptionFeature or enhancement
Proposal:
I came across some seemingly redundant fstat() and lseek() calls when working on a tool that scanned a directory of lots of small YAML files and loaded their contents as config. In tracing I found most execution time wasn't in the python interpreter but system calls (on top of NFS in that case, which made some I/O calls particularly slow).
I've been experimenting with a program that reads all .rst files in the python Docs directory to try and remove some of those redundant system calls..
Test Program
from pathlib import Path
nlines = []
for filename in Path("cpython/Doc").glob("**/*.rst"):
nlines.append(len(filename.read_text()))In my experimentation, with some tweaks to fileio can remove over 10% of the system calls the test program makes when scanning the whole Doc folders for .rst files on both macOS and Linux (don't have a Windows machine to measure on).
Current State (9 system calls)
Currently on my Linux machine to read a whole .rst file with the above code there is this series of system calls:
openat(AT_FDCWD, "cpython/Doc/howto/clinic.rst", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=343, ...}) = 0
ioctl(3, TCGETS, 0x7ffe52525930) = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_CUR) = 0
lseek(3, 0, SEEK_CUR) = 0
fstat(3, {st_mode=S_IFREG|0644, st_size=343, ...}) = 0
read(3, ":orphan:\n\n.. This page is retain"..., 344) = 343
read(3, "", 1) = 0
close(3) = 0Target State (7 5 system calls)
It would be nice to get it down to (for small files, large file caveat in PR / get an additional seek):
# Open the file
openat(AT_FDCWD, "cpython/Doc/howto/clinic.rst", O_RDONLY|O_CLOEXEC) = 3
# Check if the open fd is a file or directory and early-exit on directories with a specialized error.
# With my changes we also stash the size information from this for later use as an estimate.
fstat(3, {st_mode=S_IFREG|0644, st_size=343, ...}) = 0
# Read the data directly into a PyBytes
read(3, ":orphan:\n\n.. This page is retain"..., 344) = 343
# Read the EOF marker
read(3, "", 1) = 0
# Close the file
close(3) = 0In a number of cases (ex. importing modules) there is often a fstat followed immediately by an open / read the file (which does another fstat typically), but that is an extension point and I want to keep that out of scope for now.
Questions rattling around in my head around this
Some of these are likely better for Discourse / longer form discussion, happy to start threads there as appropriate.
- Is there a way to add a test for certain system calls happening with certain arguments and/or a certain amount of time? (I don't currently see a great way to write a test to make sure the number of system calls doesn't change unintentionally)
- Running a simple python script (
python simple.py that contains print("Hello, World!")) currently reads simple.py in full at least 4 times and does over 5 seeks. I have been pulling on that thread but it interacts with importlib as well as how the python compiler currently works, still trying to get my head around. Would removing more of those overheads be something of interest / should I keep working to get my head around it?
- We could potentially save more
- with readv (one readv call, two iovecs). I avoided this for now because _Py_read does quite a bit.
- dispatching multiple calls in parallel using asynchronous I/O APIs to meet the python API guarantees; I am experimenting with this (backed by relatively new Linux I/O APIs but possibly for kqueue and epoll), but it's very experimental and feeling a lot like "has to be a interpreter primitive" to me to work effectively which is complex to plumb through. Very early days though, many thoughts, not much prototype code.
- The
_blksize member of fileio was added in bpo-21679. It is not used much as far as I can tell as its reflection _blksize in python or in the code. The only usage I can find is https://github.com/python/cpython/blob/main/Modules/_io/_iomodule.c#L365-L374, where we could just query for it when needed in that case to save some storage on all fileio objects. The behavior of using the stat returned st_blksize is part of the docs, so doesn't feel like we can fully remove it.
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
Reacted by Collin Anderson, Victor Stinner, Yuxuan Zhang, Cody Maloney, Mikhail Efimov, Kennedy Kori, Saurabh Misra and EDM115 Activitychanged the title [-]Speed up open().readall() pattern by reducing the number of system calls[/-] [+]Speed up open().read() pattern by reducing the number of system calls[/+] on Jun 19, 2024mdboom commented on Jun 20, 2024
Running a simple python script (python simple.py that contains print("Hello, World!")) currently reads simple.py in full at least 4 times and does over 5 seeks. I have been pulling on that thread but it interacts with importlib as well as how the python compiler currently works, still trying to get my head around. Would removing more of those overheads be something of interest / should I keep working to get my head around it?
Startup time is always an important thing to optimize for. cmaloney commented on Jun 21, 2024 ContributorAuthor More actions I found the source of the remaining lseek(3, 0, SEEK_CUR) by using lldb to set a break point and get a backtrace. In _io.open() (_io_open_impl), by default, buffering is on, and that results in a BufferedIO* object being constructed. All of those call _buffered_init which calls _buffered_raw_tell to figure out the position so the buffered IO can track positions itself.
In the read whole file case that seems to be a lot of unnecessary work (Allocate a additional buffer and lock, do the seek, a list of "chunks" for read that get joined w/ special fast cases for "only one chunk", etc) which gets discarded shortly after. While it is likely possible to rewrite read_bytes() and read_text() to do less work, open().read() is common enough in code I'm planning to try and make that do less work overall, trying to avoid special-casing the operations. cmaloney commented on Jun 27, 2024 ContributorAuthor More actions Summary
# main
read_file_large: Mean +- std dev: 24.0 us +- 0.4 us
# readall_faster, removes a stat (#120755):
read_file_large: Mean +- std dev: 21.2 us +- 0.6 us
# No isatty call (no pr yet)
read_file_large: Mean +- std dev: 20.2 us +- 0.3 us No patch that removes the BufferedIO/TextIO seek passes tests yet. Planning to see what happens if stash whole stat result rather than lots of individual members.
Details
It touches a lot of code to remove the remaining seek. BufferedIO has implementation for the "absolute position" abs_pos to be optional / lazily filled. Once that is removed however, TextIO checks for "is this seekable" on construction, which is a boolean member that must be filled at construction currently. Still trying for a well contained way to do that.
The isatty call can in theory have a shortcut added around it using the fstat st_mode information. Just adding a "if this is a regular file" check before it + member accessible from fileio I get a speedup on my Mac :
main
python ../benchmark.py
.....................
read_file_small: Mean +- std dev: 8.43 us +- 0.12 us
.....................
read_file_large: Mean +- std dev: 24.0 us +- 0.4 us
cmaloney/readall_faster (#120755)
.....................
read_file_small: Mean +- std dev: 7.92 us +- 0.07 us
.....................
read_file_large: Mean +- std dev: 21.2 us +- 0.6 us
No isatty (local only currently):
.....................
read_file_small: Mean +- std dev: 7.32 us +- 0.09 us
.....................
read_file_large: Mean +- std dev: 20.2 us +- 0.3 us
That reduces the time for reading a single large .rst file in the cpython repo ~15% so far.
I built a little larger benchmark to more match code I ran into this with: "read all the .rst and .py in the cpython repo"
import pyperf
from pathlib import Path
def read_all(all_paths):
for p in all_paths:
p.read_bytes()
def read_file(path_obj):
path_obj.read_text()
all_rst = list(Path("Doc").glob("**/*.rst"))
all_py = list(Path(".").glob("**/*.py"))
assert all_rst, "Should have found .rst files"
assert all_py, "Should have found .py source files"
runner = pyperf.Runner()
runner.bench_func("read_file_small", read_file, Path("Doc/howto/clinic.rst"))
runner.bench_func("read_file_large", read_file, Path("Doc/c-api/typeobj.rst"))
runner.bench_func("read_all_rst", read_all, all_rst)
runner.bench_func("read_all_py", read_all, all_py)And on my Linux box read_all_py is ~9% faster with both changes at the moment.
I'm iterating on top of my existing PR code. Both removing the extra fstat and isatty require getting and stashing more data out of the initial stat result, so planning to see what the code looks like to just stash the whole stat result (although it is fairly big, so possibly with a hatch to unset it on write/seek or manually). This would also be a way to remove the _blksize member which is also only used at open() time (to figure out buffer size to pass to BufferedIO*). cmaloney commented on Jul 4, 2024 ContributorAuthor More actions Could this be reopened? My plan is to keep working on trying to simplify what happens in this case, would like to attach to this issue (strace pr followup test, removing seeks, possibly removing isatty) cmaloney commented on Nov 4, 2024 ContributorAuthor More actions Thanks for your help in reviewing, helping me learn CPython, and getting the patches landed; sorry there were quite so many follow up patches, but think we got there :). Was measuring 3.13 vs. 3.14 main "read a whole file" performance and it's generally between 10-15% reading OS cached files on MacOS and Linux for a LTO optimized build. Nice little wins while keeping overall API guarantees / stability :)
Details
Python 3.14 commit: commit 7d7d56d (HEAD -> main, origin/main, origin/HEAD)
Python 3.13 commit: commit 60403a5 (tag: v3.13.0, v3.13.0)
Python 3.13
(pyperf_313_env) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 6.21 us +- 0.10 us
.....................
read_file_large: Mean +- std dev: 18.1 us +- 0.6 us
.....................
read_all_rst_bytes: Mean +- std dev: 1.14 ms +- 0.01 ms
.....................
read_all_py_bytes: Mean +- std dev: 10.8 ms +- 0.1 ms
.....................
read_all_rst_text: Mean +- std dev: 2.09 ms +- 0.05 ms
(pyperf_313_env) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 6.22 us +- 0.09 us
.....................
read_file_large: Mean +- std dev: 17.2 us +- 1.1 us
.....................
read_all_rst_bytes: Mean +- std dev: 1.10 ms +- 0.01 ms
.....................
read_all_py_bytes: Mean +- std dev: 11.7 ms +- 0.4 ms
.....................
read_all_rst_text: Mean +- std dev: 2.00 ms +- 0.05 ms
(pyperf_313_env) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 6.06 us +- 0.08 us
.....................
read_file_large: Mean +- std dev: 17.0 us +- 0.6 us
.....................
read_all_rst_bytes: Mean +- std dev: 1.10 ms +- 0.02 ms
.....................
read_all_py_bytes: Mean +- std dev: 11.8 ms +- 0.2 ms
.....................
read_all_rst_text: Mean +- std dev: 2.06 ms +- 0.06 ms
(pyperf_313_env) ➜ cpython git:(heads/v3.13.0) python --version
Python 3.13.0
Python 3.14a0+ (gh-120754 done)
(pyperf_314_env) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 5.04 us +- 0.15 us
.....................
read_file_large: Mean +- std dev: 15.1 us +- 1.0 us
.....................
read_all_rst_bytes: Mean +- std dev: 820 us +- 32 us
.....................
read_all_py_bytes: Mean +- std dev: 9.31 ms +- 0.24 ms
.....................
read_all_rst_text: Mean +- std dev: 1.76 ms +- 0.08 ms
(pyperf_314_env) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 5.02 us +- 0.08 us
.....................
read_file_large: Mean +- std dev: 14.6 us +- 0.6 us
.....................
read_all_rst_bytes: Mean +- std dev: 811 us +- 12 us
.....................
read_all_py_bytes: Mean +- std dev: 9.33 ms +- 0.15 ms
.....................
read_all_rst_text: Mean +- std dev: 1.75 ms +- 0.06 ms
(pyperf_314_env) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 5.02 us +- 0.08 us
.....................
read_file_large: Mean +- std dev: 15.0 us +- 0.8 us
.....................
read_all_rst_bytes: Mean +- std dev: 809 us +- 12 us
.....................
read_all_py_bytes: Mean +- std dev: 9.29 ms +- 0.09 ms
.....................
read_all_rst_text: Mean +- std dev: 1.74 ms +- 0.02 ms
(pyperf_314_env) ➜ cpython git:(main) python --version
Python 3.14.0a1+
MacOS
MacOS v3.13.0
(pyperf_313_venv) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 9.34 us +- 0.26 us
.....................
read_file_large: Mean +- std dev: 22.3 us +- 0.6 us
.....................
read_all_rst: Mean +- std dev: 2.87 ms +- 0.03 ms
.....................
read_all_py: Mean +- std dev: 20.8 ms +- 0.3 ms
(pyperf_313_venv) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 9.30 us +- 0.17 us
.....................
read_file_large: Mean +- std dev: 21.9 us +- 0.2 us
.....................
read_all_rst: Mean +- std dev: 2.87 ms +- 0.03 ms
.....................
read_all_py: Mean +- std dev: 20.8 ms +- 0.2 ms
(pyperf_313_venv) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 9.22 us +- 0.10 us
.....................
read_file_large: Mean +- std dev: 21.8 us +- 0.2 us
.....................
read_all_rst: Mean +- std dev: 2.87 ms +- 0.04 ms
.....................
read_all_py: Mean +- std dev: 20.8 ms +- 0.5 ms
(pyperf_313_venv) ➜ cpython git:(heads/v3.13.0) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 9.25 us +- 0.07 us
.....................
read_file_large: Mean +- std dev: 21.8 us +- 0.2 us
.....................
read_all_rst: Mean +- std dev: 2.86 ms +- 0.03 ms
.....................
read_all_py: Mean +- std dev: 20.8 ms +- 0.3 ms
MacOS v3.14.0a0+ commit 34653bb (HEAD -> main, origin/main, origin/HEAD)
(pyperf_314_venv) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 8.18 us +- 0.08 us
.....................
read_file_large: Mean +- std dev: 21.1 us +- 0.3 us
.....................
read_all_rst: Mean +- std dev: 2.58 ms +- 0.05 ms
.....................
read_all_py: Mean +- std dev: 19.0 ms +- 0.2 ms
(pyperf_314_venv) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 8.17 us +- 0.11 us
.....................
read_file_large: Mean +- std dev: 21.1 us +- 0.3 us
.....................
read_all_rst: Mean +- std dev: 2.57 ms +- 0.03 ms
.....................
read_all_py: Mean +- std dev: 19.1 ms +- 0.3 ms
(pyperf_314_venv) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 8.16 us +- 0.07 us
.....................
read_file_large: Mean +- std dev: 21.2 us +- 0.3 us
.....................
read_all_rst: Mean +- std dev: 2.58 ms +- 0.05 ms
.....................
read_all_py: Mean +- std dev: 19.0 ms +- 0.2 ms
(pyperf_314_venv) ➜ cpython git:(main) python ../bm_readall.py
.....................
read_file_small: Mean +- std dev: 8.20 us +- 0.07 us
.....................
read_file_large: Mean +- std dev: 21.1 us +- 0.3 us
.....................
read_all_rst: Mean +- std dev: 2.58 ms +- 0.03 ms
.....................
read_all_py: Mean +- std dev: 19.1 ms +- 0.4 ms
import pyperf
from pathlib import Path
all_rst = list(Path("Doc").glob("**/*.rst"))
all_py = list(Path(".").glob("**/*.py"))
assert all_rst, "Should have found rst files. Is the current directory a cpython checkout?"
assert all_py, "Should have found python source files. Is the current directory a cpython checkout?"
def read_all_bytes(all_paths):
for p in all_paths:
p.read_bytes()
def read_all_text(all_paths):
for p in all_paths:
p.read_text()
def read_file(path_obj):
path_obj.read_text()
def read_bytes(path_obj):
path_obj.read_bytes()
# print(f"all_rst={len(all_rst)}|all_py={len(all_py)}")
# Make a fixed number across python versions.
# File sizes may still vary a bit, but this helps a lot for stability,
all_rst = all_rst[:200]
all_py = all_py[:3000]
assert len(all_rst) == 200
assert len(all_py) == 3000
runner = pyperf.Runner()
runner.bench_func('read_file_small', read_file, Path("Doc/howto/clinic.rst"))
runner.bench_func('read_file_large', read_file, Path("Doc/c-api/typeobj.rst"))
runner.bench_func('read_all_rst_bytes', read_all_bytes, all_rst)
runner.bench_func('read_all_py_bytes', read_all_bytes, all_py)
# NOTE: No test to read all `.py` as text as some contains invalid utf-8 byte sequences.
runner.bench_func('read_all_rst_text', read_all_text, all_rst)Reacted by Collin Anderson, Erlend E. Aasland, Shantanu, Maxim Martynov, Victor Stinner, Kirill Podoprigora, Jakub Beránek, Mikhail Efimov and Kennedy Kori vstinner commented on Nov 5, 2024 Reacted by Erlend E. Aasland and Cody Maloney
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|