-
-
Notifications
You must be signed in to change notification settings - Fork 33k
gh-138535: Optimize fill_time for typical timestamps #138537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
While file timestamps can be anything the file system can store, most lie between the recent past and the near future. Optimize fill_time for typical timestamps in three ways: - When possible, convert to nanoseconds with C arithmetic. - When using C arithmetic and the seconds member is not required (for st_birthtime), avoid creating a long object. - When using C arithmetic, reorder the code to avoid the null checks implied in Py_XDECREF.
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
I want to add tests for timestamps just inside and outside the fast path to After some reflection, we can deal with varying Another way is to parse |
I don't think that checking large timestamp support using the filesystem magic number or name is actually that useful. I think tmpfs has supported 64-bit timestamps as long as the kernel VFS layer has. btrfs has always had 64-bit timestamps on-disk but I'm not sure they were plumbed in right away. The in-kernel NTFS driver should also have 64-bit timestamps, but I think most Linux NTFS users use the FUSE implementation ntfs-3g, and that will have the same magic number as other FUSE filesystems. For the other common Linux filesystems we can't assume large timestamps. It's still possible to format ext4 with 32-bit timestamps, and anyway ext2 and ext3 use the same I imagine most programs that care test for large timestamps by seeing if utimes and friends fails or stat returns a clamped timestamp ("Updated file timestamps are set to the greatest value supported by the filesystem that is not greater than the specified time." -- So, reviewers, would you be satisfied just testing this on Windows NTFS by adding a few timestamps to |
For the tests I would prefer to keep it simple: just add some more cases above and below the fast path threshold. If needed add tests for both 32- and 64-bit. There is a slight behavior change with this PR (I do not think it matters, but that is for someone else to judge): with current main if an error occurs anywhere, no changes are made to the struct sequence. In this PR it can happen that some changes are already made (e.g. @jbosboom Could you add a news entry? |
Co-authored-by: Victor Stinner <[email protected]>
I re-run the benchmark on Python built with
Nice performance improvement! |
Merged, thanks for this nice optimization. |
…8537) While file timestamps can be anything the file system can store, most lie between the recent past and the near future. Optimize fill_time() for typical timestamps in three ways: - When possible, convert to nanoseconds with C arithmetic. - When using C arithmetic and the seconds member is not required (for st_birthtime), avoid creating a long object. - When using C arithmetic, reorder the code to avoid the null checks implied in Py_XDECREF(). Co-authored-by: Victor Stinner <[email protected]>
While file timestamps can be anything the file system can store, most lie between the recent past and the near future. Optimize fill_time for typical timestamps in three ways:
This improves
python -m pyperf timeit -s 'import os' 'os.stat(".")'
from1.26 us +- 0.02 us
to1.15 us +- 0.01 us
on Linux 6.16.2.arch1-1 btrfs and--enable-optimizations --with-lto
.I found this while implementing
os.stat_result.st_birthtime
on Linux and trying not to regress performance. This is a small change that should be an improvement on all platforms, so I've submitted it separately.This needs tests, presumably in
test_os.py
UtimeTests.test_large_time
, though it's actually testingos.stat
. But that test currently only runs on NTFS on Windows. I'll look at Linux filesystem support tomorrow.