Skip to content

Speed up open().read() pattern by reducing the number of system calls #120754

Closed
#120755
Closed
Speed up open().read() pattern by reducing the number of system calls#120754
#120755
Labels
performancePerformance or resource usagetype-featureA feature request or enhancement

Description

@cmaloney
cmaloney
opened on Jun 19, 2024

Feature 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)                                = 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 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)                                = 0

In 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.

  1. 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)
  2. 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?
  3. We could potentially save more
    1. with readv (one readv call, two iovecs). I avoided this for now because _Py_read does quite a bit.
    2. 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.
  4. 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

Activity

  1. cmaloney
    added
    type-featureA feature request or enhancement
    on Jun 19, 2024
  2. cmaloney
    changed 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, 2024
  3. mdboom
    added
    performancePerformance or resource usage
    on Jun 20, 2024

  4. mdboom commented on Jun 20, 2024

    @mdboom
    mdboom
    on Jun 20, 2024
    Contributor

    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.

  5. cmaloney commented on Jun 21, 2024

    @cmaloney
    cmaloney
    on Jun 21, 2024
    ContributorAuthor

    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.

  6. cmaloney commented on Jun 27, 2024

    @cmaloney
    cmaloney
    on Jun 27, 2024
    ContributorAuthor

    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*).

  7. vstinner
    added a commit that references this issue on Jul 4, 2024

    gh-120754: Reduce system calls in full-file FileIO.readall() case (#1…

    2f5f19e
  8. vstinner
    closed this as completedin #120755on Jul 4, 2024

  9. cmaloney commented on Jul 4, 2024

    @cmaloney
    cmaloney
    on Jul 4, 2024
    ContributorAuthor

    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)

  10. vstinner
    reopened this on Jul 4, 2024
  11. cmaloney
    added a commit that references this issue on Jul 4, 2024

    pythongh-120754: Update size_estimated in C truncate

    b941f55
  12. vstinner
    added a commit that references this issue on Jul 4, 2024

    gh-120754: Update estimated_size in C truncate (#121357)

    06a1c3f
  13. cmaloney
    added 3 commits that reference this issue on Jul 10, 2024

    pythongh-120754 Remove isatty call during regular read

    0bb809f

    pythonGH-120754: Remove isatty call during regular read

    c2e86c4

    pythonGH-120754: Remove isatty call during regular read

    e50c5a0

  14. 75 remaining items

  15. cmaloney commented on Nov 4, 2024

    @cmaloney
    cmaloney
    on Nov 4, 2024
    ContributorAuthor

    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)

  16. vstinner commented on Nov 5, 2024

    @vstinner
    vstinner
    on Nov 5, 2024
    Member

    Maybe you should document this optimization at: https://docs.python.org/dev/whatsnew/3.14.html#optimizations

  17. cmaloney
    added 2 commits that reference this issue on Nov 6, 2024

    pythongh-120754: Add to `io` optimization to What's new

    9edd3fa

    pythongh-120754: Add to `io` optimization to What's new

    daf18c5
  18. vstinner
    added a commit that references this issue on Nov 6, 2024

    gh-120754: Add to `io` optimization to what's new (#126466)

    6431f37
  19. picnixz
    added 4 commits that reference this issue on Dec 8, 2024

    pythongh-120754: _io Ensure stat cache is cleared on fd change (pytho…

    ed81044

    pythongh-120754: Add a strace helper and test set of syscalls for ope…

    eba7214

    pythongh-113977, pythongh-120754: Remove unbounded reads from zipfile (…

    b7046ab

    pythongh-120754: Add to `io` optimization to what's new (python#126466)

    8c6dc67
  20. ebonnal
    added 4 commits that reference this issue on Jan 12, 2025

    pythongh-120754: _io Ensure stat cache is cleared on fd change (pytho…

    167302c

    pythongh-120754: Add a strace helper and test set of syscalls for ope…

    b2cbc72

    pythongh-113977, pythongh-120754: Remove unbounded reads from zipfile (…

    22db78f

    pythongh-120754: Add to `io` optimization to what's new (python#126466)

    a994049
  21. cmaloney
    added a commit that references this issue on Sep 15, 2025

    pythongh-129813, PEP 782: Use PyBytesWriter in _io.FileIO.readall

    86b9f6b
  22. vstinner
    added a commit that references this issue on Sep 15, 2025

    gh-129813, PEP 782: Use PyBytesWriter in _io.FileIO.readall (#138901)

    71defb6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancePerformance or resource usagetype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions