perf(capture): skip reading the capture file when nothing was written#14687
Open
henryiii wants to merge 2 commits into
Open
perf(capture): skip reading the capture file when nothing was written#14687henryiii wants to merge 2 commits into
henryiii wants to merge 2 commits into
Conversation
FDCapture.snap() did seek/read/seek/truncate through the TextIOWrapper on every test phase even when the test produced no output. Since the tmpfile is unbuffered, a single fstat can prove it is empty and skip all of that. For suites of many small quiet tests this removes ~6 of these cycles per test (2 streams x 3 phases); on pypa/packaging (62k tests) it cuts the total runtime by about 8%. Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
Assisted-by: ClaudeCode:claude-fable-5
psf-chronographer
Bot
added
the
bot:chronographer:provided
Member
That's substantial. I'm surprised those seek/truncate calls are that "expensive" on an empty file.
Yes that's perfectly fine, thanks. |
nicoddemus
approved these changes
Jul 7, 2026
Pierre-Sassoulas
approved these changes
Jul 7, 2026
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
Member
There was a problem hiding this comment.
Looks great, I can reproduce locally, looking at the cprofile, it seems like it also eliminate calls to <method 'truncate' of '_io.TextIOWrapper' objects>. It's small gain for pytest that has a few assert True calls (0.24% probably within noise) but become significant if you construct code that exercise the costly path.
Pierre-Sassoulas
added
the
type: performance
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
4 participants
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.
I was investigating pytest performance on pypa/packaging's test suite, which has a lot of small tests, and pytest takes quite a bit of time. I used python 3.15's new sampling profiler, setup something like this:
I then feed the report into Claude and had it look for spots that could be made faster. This is the first thing it found.
FDCapture.snap()didseek/read/seek/truncateeven when the test produced no output. Since the tmpfile is unbuffered, a singlefstatcan prove it is empty and skip all of that.This removes ~6 of these cycles per test (2 streams x 3 phases); on pypa/packaging (62k tests) it cuts the total runtime by about 8% (the overhead from pytest something like 25%, I think).
I used an
Assisted-bytrailer (as recommended by the linux kernel) instead of aCo-authored-bytrailer, I can switch if you prefer; generally Co-authored-by seems to be discouraged due to the implied copyright ownership. I rewrote the comments, so it's 33/67 me and Claude.There isn't an associated issue, so will do a changelog with the PR number if that's fine.
Assisted-by: ClaudeCode:claude-fable-5