Skip to content
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

Add fast path to os.[l]stat() that returns incomplete information #99726

Closed
zooba opened this issue Nov 23, 2022 · 8 comments
Closed

Add fast path to os.[l]stat() that returns incomplete information #99726

zooba opened this issue Nov 23, 2022 · 8 comments
Labels
3.12 bugs and security fixes OS-windows performance Performance or resource usage

Comments

@zooba
Copy link
Member

zooba commented Nov 23, 2022

A future update to Windows is bringing a new filesystem API for getting stat(-like) information more efficiently from a filename. Currently, we have to open the file, which is quite a slow operation. Being able to simply request metadata based on the path is a real improvement. My testing shows os.stat() and os.lstat() (in the case where no traversal is needed) taking less than 1/4 of their current time when using the new API. I'll link the change in a PR below.

However, the new API does not include the volume serial number, which is how we fill in the st_dev field. Adding an additional call to get the VSN takes all the time we were taking before, so there's no performance benefit.1

So I'd like to propose adding a fast=False argument to os.stat and os.lstat. When left as False, you get the current behaviour. If you pass True, we only guarantee a smaller set of data, and warn that other fields may be absent on some platforms.

Looking through the fields, I have proposed that the file type bits of st_mode (not permissions), the st_size and st_mtime[_ns] fields are the only ones that are important to guarantee.2 All the rest can stay as they are, but we then have the option to drop them from the fast path in the future.3 It's no accident that these are the APIs we already offer as other os.path functions (apart from samestat, which will have to stay on the slow path and probably needs an even slower check in order to be x-plat reliable...)

I'm not sure who cares most about this, so I'm going to leave this open for a while.

Linked PRs

Footnotes

  1. There is still discussion about changing this API before it releases. If that happens, the rest of this proposal is moot, unless we like the idea anyway.

  2. On Windows, we can further guarantee st_file_attributes and st_reparse_tag, as these are the raw values used to calculate the file type bits of st_mode.

  3. stat is already very fast on POSIX-ish filesystems, so it's unlikely to be an issue there, but if we wanted to specialise for network FS or similar then we'd be able to.

@zooba zooba added performance Performance or resource usage OS-windows 3.12 bugs and security fixes labels Nov 23, 2022
zooba added a commit to zooba/cpython that referenced this issue Nov 23, 2022
When passed as True, only st_mode's type bits, st_size and st_mtime[_nsec] are guaranteed to be set. Other fields may also be set for a given Python version on a given platform version, but may change without warning (in the case of OS changes - Python will try to keep them stable).
This first implementation uses a new Windows API that is significantly faster, provided the volume identifier is not required. Other optimizations may be added later.
@zooba
Copy link
Member Author

zooba commented Nov 23, 2022

Updating importlib has given a ~5-10% improvement for python -c "import asyncio, typing, urllib.request" (with all .pyc files already cached) on Windows builds with the new API.

@eryksun
Copy link
Contributor

eryksun commented Nov 24, 2022

I'm familiar with NTAPI NtQueryInformationByName(), which currently supports FileStatInformation and FileCaseSensitiveInformation. Apparently, the Windows API will expose them as FileStatByNameInfo and FileCaseSensitiveByNameInfo, as well as a new FileStatBasicByNameInfo class.

If you can, please ask the them to also expose FileStatInfo and FileStatBasicInfo for use with GetFileInformationByHandleEx(). Even if the basic stat info lacks the volume serial number, I'd rather combine FileStatBasicInfo with GetVolumeInformationByHandleW() -- than combine FileAttributeTagInfo with GetFileInformationByHandle().

the new API does not include the volume serial number, which is how we fill in the st_dev field

The filesystem has nearly zero-cost access to the volume serial number. In the NT API, it's available in FileFsVolumeInformation as VolumeSerialNumber. This info class also includes the volume name (label) and volume creation time, but those aren't important for a stat result.

Another value that could be useful is FileSystemAttributes (e.g. FILE_READ_ONLY_VOLUME, FILE_PERSISTENT_ACLS, FILE_SUPPORTS_REPARSE_POINTS, FILE_SUPPORTS_HARD_LINKS, etc) from FileFsAttributeInformation. This info class also includes the filesystem name (e.g. "Ntfs") and maximum component length (e.g. 255 characters), but those aren't broadly useful.

@eryksun
Copy link
Contributor

eryksun commented Nov 24, 2022

The basic stat info doesn't include the EffectiveAccess field that FileStatInformation includes. Instead it has DeviceType and DeviceCharacteristics. These are mostly likely based on NTAPI FileFsDeviceInformation, the info class that's used by WinAPI GetFileType() and GetDriveTypeW().

Based on your code, they're applying a file-type categorization to the DeviceType field. NT has about 90 device types (e.g. FILE_DEVICE_DISK, FILE_DEVICE_CD_ROM, FILE_DEVICE_CONSOLE, FILE_DEVICE_NULL). GetFileType() categorizes the NT device type of an open file as one of 4 file types: FILE_TYPE_DISK, FILE_TYPE_CHAR, FILE_TYPE_PIPE, and FILE_TYPE_UNKNOWN.

There are several useful device characteristics, including FILE_REMOVABLE_MEDIA, FILE_READ_ONLY_DEVICE, FILE_REMOTE_DEVICE, FILE_DEVICE_IS_MOUNTED, FILE_PORTABLE_DEVICE, and FILE_CHARACTERISTIC_WEBDAV_DEVICE1.

WinAPI GetDriveTypeW() returns DRIVE_REMOTE2 if the device has the FILE_REMOTE_DEVICE characteristic. It returns DRIVE_REMOVABLE if a regular disk device (i.e. not a CD-ROM or RAM disk) has the FILE_REMOVABLE_MEDIA characteristic. Otherwise it returns one of DRIVE_FIXED, DRIVE_CDROM, DRIVE_RAMDISK, or DRIVE_UNKNOWN based on the NT device type.

It would be nice to expose the device characteristics as st_device_characteristics.

Footnotes

  1. WebDAV remote devices (e.g. accessing "\\live.sysinternals.com\tools" on Windows) are a pain because the WebDAV driver doesn't implement a volume serial number (st_dev) or file IDs (st_ino).

  2. This is useful to know, for example, because the target path of a junction in a remote path is just a local path on the remote system (e.g. "C:\spam\eggs" on the server's "C:" drive). It cannot be followed manually, such as by realpath() in non-strict mode.

@eryksun
Copy link
Contributor

eryksun commented Nov 24, 2022

Regarding the ChangeTime field in FileStatByNameInfo and FileStatBasicByNameInfo, Python's st_ctime on Windows has always been defined incorrectly according to POSIX. It's supposed to be the change time, not the creation time. Some Unix systems return the creation time as st_birthtime or st_btime.

Currently, the change time could be obtained in stat() via GetFileInformationByHandleEx(): FileBasicInfo. In os.scandir(), the change time could be obtained by listing the directory via GetFileInformationByHandleEx() with FileFullDirectoryInfo (the fastest directory listing that returns basic stat info) or FileIdBothDirectoryInfo.

@eryksun
Copy link
Contributor

eryksun commented Nov 24, 2022

Related idea:

Given FileStatInfo is available and the non-basic FileStatByNameInfo has an EffectiveAccess field, we could use them to implement os.access(path, mode, effective_ids, follow_symlinks). If effective_ids is false and the thread is impersonating, temporarily clear the thread's impersonation token (i.e. revert to self).

@clintonroy
Copy link

The statx call has a reasonably nice api, in that you pass along the list of attributes you're interested in, maybe we could mirror that, and the implementation could work out what system call to use?

@zooba
Copy link
Member Author

zooba commented Nov 24, 2022

The statx call has a reasonably nice api

Now that's an idea. https://man7.org/linux/man-pages/man2/statx.2.html for reference. I've got two days of work with nobody else around (US Thanksgiving), so maybe I'll have a go at adding this API instead.

zooba added a commit to zooba/cpython that referenced this issue Nov 24, 2022
zooba added a commit to zooba/cpython that referenced this issue Nov 25, 2022
zooba added a commit to zooba/cpython that referenced this issue Nov 28, 2022
zooba added a commit to zooba/cpython that referenced this issue Dec 5, 2022
carljm added a commit to zooba/cpython that referenced this issue Dec 15, 2022
zooba added a commit to zooba/cpython that referenced this issue Feb 22, 2023
zooba added a commit to zooba/cpython that referenced this issue Feb 28, 2023
zooba added a commit to zooba/cpython that referenced this issue Mar 3, 2023
zooba added a commit to zooba/cpython that referenced this issue Mar 6, 2023
zooba added a commit to zooba/cpython that referenced this issue Mar 9, 2023
zooba added a commit to zooba/cpython that referenced this issue Mar 14, 2023
zooba added a commit that referenced this issue Mar 16, 2023
…faster API when available (GH-102149)

This deprecates `st_ctime` fields on Windows, with the intent to change them to contain the correct value in 3.14. For now, they should keep returning the creation time as they always have.
@zooba
Copy link
Member Author

zooba commented Mar 16, 2023

Implemented, without returning incomplete information! The Windows team updated their API to include everything we needed, so it's just a transparent perf improvement now (as well as a minor correctness improvement).

@zooba zooba closed this as completed Mar 16, 2023
carljm added a commit to carljm/cpython that referenced this issue Mar 17, 2023
* main: (34 commits)
  pythongh-102701: Fix overflow in dictobject.c (pythonGH-102750)
  pythonGH-78530: add support for generators in `asyncio.wait` (python#102761)
  Increase stack reserve size for Windows debug builds to avoid test crashes (pythonGH-102764)
  pythongh-102755: Add PyErr_DisplayException(exc) (python#102756)
  Fix outdated note about 'int' rounding or truncating (python#102736)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (python#102760)
  pythongh-99726: Improves correctness of stat results for Windows, and uses faster API when available (pythonGH-102149)
  pythongh-102192: remove redundant exception fields from ssl module socket (python#102466)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (python#102743)
  pythongh-102737: Un-ignore ceval.c in the CI globals check (pythongh-102745)
  pythonGH-102748: remove legacy support for generator based coroutines from `asyncio.iscoroutine` (python#102749)
  pythongh-102721: Improve coverage of `_collections_abc._CallableGenericAlias` (python#102722)
  pythonGH-102653: Make recipe docstring show the correct distribution (python#102742)
  Add comments to `{typing,_collections_abc}._type_repr` about each other (python#102752)
  pythongh-102594: PyErr_SetObject adds note to exception raised on normalization error (python#102675)
  pythongh-94440: Fix issue of ProcessPoolExecutor shutdown hanging (python#94468)
  pythonGH-100112:  avoid using iterable coroutines in asyncio internally (python#100128)
  pythongh-102690: Use Edge as fallback in webbrowser instead of IE (python#102691)
  pythongh-102660: Fix Refleaks in import.c (python#102744)
  pythongh-102738: remove from cases generator the code related to register instructions (python#102739)
  ...
zooba added a commit to zooba/cpython that referenced this issue Mar 23, 2023
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
… uses faster API when available (pythonGH-102149)

This deprecates `st_ctime` fields on Windows, with the intent to change them to contain the correct value in 3.14. For now, they should keep returning the creation time as they always have.
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
… uses faster API when available (pythonGH-102149)

This deprecates `st_ctime` fields on Windows, with the intent to change them to contain the correct value in 3.14. For now, they should keep returning the creation time as they always have.
warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes OS-windows performance Performance or resource usage
Projects
None yet
Development

No branches or pull requests

3 participants