| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Co-authored-by: Eryk Sun <eryksun@gmail.com>
…lexists()` Use `os.path.lexists()` rather than `os.lstat()` to test whether paths exist. This is equivalent on POSIX, but faster on Windows.
|
I'm working on what I hope will be an improved version compared to the first draft. AFAIK, the GetFileInformationByName() fast path will be generally available when Windows 11 24H2 is released later this year. Until then, and for older systems, we need to focus on improving the 'slow' path. I'm looking to avoid full STAT and LSTAT calls, except as a last resort. |
Sorry, something went wrong.
|
Here's the revised implementation of nt_exists(): static PyObject *
nt_exists(PyObject *path, int follow_symlinks)
{
path_t _path = PATH_T_INITIALIZE("exists", "path", 0, 1);
HANDLE hfile;
BOOL traverse = follow_symlinks;
int result = 0;
if (!path_converter(path, &_path)) {
path_cleanup(&_path);
if (PyErr_ExceptionMatches(PyExc_ValueError)) {
PyErr_Clear();
Py_RETURN_FALSE;
}
return NULL;
}
Py_BEGIN_ALLOW_THREADS
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
if (hfile != INVALID_HANDLE_VALUE) {
result = 1;
}
}
else if (_path.wide) {
BOOL slow_path = TRUE;
FILE_STAT_BASIC_INFORMATION statInfo;
if (_Py_GetFileInformationByName(_path.wide, FileStatBasicByNameInfo,
&statInfo, sizeof(statInfo)))
{
if (!(statInfo.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) ||
!follow_symlinks &&
IsReparseTagNameSurrogate(statInfo.ReparseTag))
{
slow_path = FALSE;
result = 1;
}
else {
// reparse point but not name-surrogate
traverse = TRUE;
}
}
else if (_Py_GetFileInformationByName_ErrorIsTrustworthy(
GetLastError()))
{
slow_path = FALSE;
}
if (slow_path) {
BOOL traverse = follow_symlinks;
if (!traverse) {
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT |
FILE_FLAG_BACKUP_SEMANTICS, NULL);
if (hfile != INVALID_HANDLE_VALUE) {
FILE_ATTRIBUTE_TAG_INFO info;
if (GetFileInformationByHandleEx(hfile,
FileAttributeTagInfo, &info, sizeof(info)))
{
if (!(info.FileAttributes &
FILE_ATTRIBUTE_REPARSE_POINT) ||
IsReparseTagNameSurrogate(info.ReparseTag))
{
result = 1;
}
else {
// reparse point but not name-surrogate
traverse = TRUE;
}
}
else {
// device or legacy filesystem
result = 1;
}
CloseHandle(hfile);
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (!LSTAT(_path.wide, &st)) {
result = 1;
}
}
}
}
if (traverse) {
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
if (hfile != INVALID_HANDLE_VALUE) {
CloseHandle(hfile);
result = 1;
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (!STAT(_path.wide, &st)) {
result = 1;
}
}
}
}
}
}
Py_END_ALLOW_THREADS
path_cleanup(&_path);
if (result) {
Py_RETURN_TRUE;
}
Py_RETURN_FALSE;
} |
Sorry, something went wrong.
Co-authored-by: Eryk Sun <eryksun@gmail.com>
|
Are you happy with the new implementation, or do you have more ideas? |
Sorry, something went wrong.
|
The performance for existent files has already greatly improved. Nice job! |
Sorry, something went wrong.
|
That's all I have for now. |
Sorry, something went wrong.
There was a problem hiding this comment.
At first glance looks correct, although I am not happy that we added so much complicated code for pure optimization of functions which should not be used in performance critical code (os.scandir() should be used instead).
Sorry, something went wrong.
The performance of os.stat() and os.lstat() will improve on Windows once GetFileInformationByName() is generally supported, starting with Windows 11 24H2 later this year. Eventually I expect the builtin _path_* tests to be removed when Python stops supporting Windows 10. A disappointment with these builtin functions is that they can't be leveraged in pathlib.Path because they're designed to simply return True or False without raising OSError exceptions. The problem is that the pathlib.Path methods exists(), is_dir(), is_file(), and is_symlink() only ignore OSError exceptions for a small set of errno values and Windows error codes. Regarding scandir(), in principle, the C implementations of _path_isdir() and _path_isfile() could be used by the DirEntry methods is_dir() and is_file(). These methods have to call STAT() if the entry is a reparse point, except if follow_symlinks is false and it's a name-surrogate reparse point1. That's more work than necessary. However, the DirEntry tests only handle FileNotFoundError exceptions, so there's a mismatch in error handling. Fortunately there usually aren't so many reparse points that the accumulated cost of STAT() calls would matter much. Footnotes
|
Sorry, something went wrong.
We can at least use it from Path.glob(), which suppresses all OSErrors. I have a draft PR for that here: #117858 I wouldn't mind if we made the is_ methods suppress all OSErrors rather than a fairly arbitrary subset as we do now. We don't document what's suppressed, and it's changed before. |
Sorry, something went wrong.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
So I'm hesitant to take this for three reasons (and these do apply to previous enhancements as well, but didn't exist at that time):
isdir and islink are useful to have, because you may glob or listdir and need to filter its members. But you don't need to exists in that case - everything in the directory has to be assumed to exist (and handle the rare race condition when you try to use it). If someone can show a scenario where you would have a significant (hundreds+) list of paths, need to check whether they exist, but couldn't use one of isfile, isdir or islink, then I could be persuaded on the third point. If there's some reason why this scenario exists for people who can upgrade Python but not update Windows, then I might be convinced on the first point (or alternatively, if lexists doesn't actually get faster with the new stat APIs, which is a possibility). I don't think we can really reduce the amount of code. If it happened to be shorter and easier to follow then I'd be less concerned about long-term maintenance, but I'm pretty sure it's as good as it gets (without adding indirection and hurting the performance again - same tradeoff we made with the earlier _path_* functions). |
Sorry, something went wrong.
On this specifically, one example would be globbing for */__init__.py. The quickest way to implement that is os.scandir() for an initial set of paths, join __init__.py onto each of them, and then call lstat() to filter out nonexistent paths. This is the approach taken in the pathlib globbing implementation: Lines 508 to 520 in a7711a2 Note that glob results can include dangling symlinks, hence lstat() rather than stat(). |
Sorry, something went wrong.
GetFileInformationByName() won't be generally available until Windows 11 24H2 later this year, right? What about older Windows 10/11 systems?
The implementation was consolidated with _path_exists() to lessen duplicate code. It was also reordered to avoid the need the need for the close_file variable. But I agree that all of these _path_is* and _path_[l]exists helpers are a lot of code to maintain, taken together. It could be refactored into smaller inline helper functions that can be reused, which would also make the code more readable. nineteendo seems to be pretty good at and enthusiastic about optimizing code. |
Sorry, something went wrong.
Sorry, something went wrong.
|
Could we simplify this? It already seems to know if it's a directory of file: Lines 5123 to 5129 in a6b610a Lines 5220 to 5226 in a6b610a |
Sorry, something went wrong.
Yes, the fast path can be simplified. If it's not a reparse point, isdir() and isfile() can always use the by-name stat information. That's implemented. isdir() can return False for a file reparse point. That's implemented. Otherwise it has to ensure that the reparsed target exists. This could be improved. Falling back to the full slow path does more work than necessary since it only needs to check existence via CreateFileW(), not the GetFileInformationByHandleEx() query. Similarly isfile() has to verify existence for a file reparse point. |
Sorry, something went wrong.
|
Tracking further in #118755. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Benchmark
script