Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
Description
Feature or enhancement
Proposal:
I came across some seemingly redundantfstat() andlseek() 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 pythonDocs directory to try and remove some of those redundant system calls..
Test Program
frompathlibimportPathnlines= []forfilenameinPath("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 wholeDoc 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)=3fstat(3, {st_mode=S_IFREG|0644,st_size=343, ...})=0ioctl(3,TCGETS,0x7ffe52525930)=-1ENOTTY (Inappropriateioctlfordevice)lseek(3,0,SEEK_CUR)=0lseek(3,0,SEEK_CUR)=0fstat(3, {st_mode=S_IFREG|0644,st_size=343, ...})=0read(3,":orphan:\n\n.. This page is retain"...,344)=343read(3,"",1)=0close(3)=0
Target 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 fileopenat(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 PyBytesread(3,":orphan:\n\n.. This page is retain"...,344)=343# Read the EOF markerread(3,"",1)=0# Close the fileclose(3)=0
In a number of cases (ex. importing modules) there is often afstat followed immediately by an open / read the file (which does anotherfstat 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.pythat containsprint("Hello, World!")) currently readssimple.pyin 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'svery 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
_blksizemember of fileio was added inbpo-21679. It is not used much as far as I can tell as its reflection_blksizein python or in the code. The only usage I can find ishttps://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 allfileioobjects. 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
- gh-120754: Reduce system calls in full-file readall case #120755
- GH-120754: Add a strace helper and test set of syscalls for open().read() #121143
- gh-120754: Update estimated_size in C truncate #121357
- GH-120754: Remove isatty call during regular open #121593
- GH-120754: Make PY_READ_MAX smaller than max byteobject size #121633
- gh-113977, gh-120754: Remove unbounded reads from zipfile #122101
- GH-120754: Add more tests around seek + readall #122103
- GH-120754: Disable buffering in Path.read_bytes #122111
- [3.13] GH-120754: Add more tests around seek + readall (GH-122103) #122215
- [3.12] GH-120754: Add more tests around seek + readall (GH-122103) #122216
- Revert "GH-120754: Add a strace helper and test set of syscalls for o… #123303
- gh-120754: Refactor I/O modules to stash whole stat result rather than individual members #123412
- gh-120754: Add a strace helper and test set of syscalls for open().read(), Take 2 #123413
- gh-120754: Fix memory leak in FileIO.__init__() #124225
- gh-120754: Ensure _stat_atopen is cleared on fd change #125166
- gh-120754: Add to
ioopen()and.read()optimization to what's new #126466