Repository navigation
Conversation
When FileIO is doing the open use one GIL release for both the open and
fstat. Passing a file descriptor or using an opener still does the
fstat separately.
readall always took the GIL at least twice: Once to read the data and a
second time through the loop for the EOF. On small files the GIL release
and reacquire was a significant portion of time. Move so the readall
loop checks for signals just as often but only releases the GIL once
before I/O and re-acquires after. _Py_read_noraise() is the platform
read() factored out of _Py_read() for callers that release the GIL
themselves, like _Py_write_noraise().
Reading a small file with FileIO(path).readall() + close(), PGO+LTO,
4 interleaved rounds of 10 processes:
| Benchmark | base | this change |
|----------------|:-------:|:---------------------:|
| fileio_readall | 3.68 us | 3.60 us: 1.02x faster |
Callgrind, instructions per call:
| Build | base | this change |
|------------------|-------:|------------:|
| PGO+LTO | 10,646 | 9,842 |
| -O3, no PGO | 14,154 | 13,303 |
<details>
<summary>Benchmarks</summary>
bm_fileio_readall.py:
```python
import io
import pyperf
def fileio_readall():
with io.FileIO(__file__) as f:
f.readall()
runner = pyperf.Runner()
runner.bench_func("fileio_readall", fileio_readall)
```
```sh
for round in 1 2 3 4; do # alternate the order between rounds
base/python bm_fileio_readall.py -p 10 -q --append base.json
change/python bm_fileio_readall.py -p 10 -q --append change.json
done
python -m pyperf compare_to base.json change.json --table
```
callgrind_readall.py:
```python
import io
import sys
for _ in range(int(sys.argv[1])):
with io.FileIO(__file__) as f:
f.readall()
```
```sh
# Instructions per call: (Ir(2000) - Ir(0)) / 2000
for n in 0 2000; do
PYTHONHASHSEED=0 valgrind --tool=callgrind --callgrind-out-file=cg.$n \
./python -S callgrind_readall.py $n
done
```
</details>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When FileIO is doing the open use one GIL release for both the open and fstat. Passing a file descriptor or using an opener still does the fstat separately.
readall always took the GIL at least twice: Once to read the data and a second time through the loop for the EOF. On small files the GIL release and reacquire was a significant portion of time. Move so the readall loop checks for signals just as often but only releases the GIL once before I/O and re-acquires after. _Py_read_noraise() is the platform read() factored out of _Py_read() for callers that release the GIL themselves, like _Py_write_noraise().
Reading a small file with FileIO(path).readall() + close(), PGO+LTO, 4 interleaved rounds of 10 processes:
Callgrind, instructions per call:
Benchmarks
bm_fileio_readall.py:
callgrind_readall.py:
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
This has a NEWS for both the changes together.