GeezerVison

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.

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;
}

Are you happy with the new implementation, or do you have more ideas?

The performance for existent files has already greatly improved. Nice job!

That's all I have for now.

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).

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 point¹. 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.

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.

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.

cc @zooba

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):

• stat is going to get fast enough on newer Windows for this optimisation to be redundant
• it's a lot of code and not easily maintainable
• I don't know any scenarios where you have a lot of paths and you need to check whether they exist

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).

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.

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:

Note that glob results can include dangling symlinks, hence lstat() rather than stat().

* `stat` is going to get fast enough on newer Windows for this optimisation to be redundant

GetFileInformationByName() won't be generally available until Windows 11 24H2 later this year, right? What about older Windows 10/11 systems?

* it's a lot of code and not easily maintainable

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.

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.

Doesn't seem too difficult for _path_isdir & _path_isfile:

Could we simplify this? It already seems to know if it's a directory of file:

Could we simplify this? It already seems to know if it's a directory of file:

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.

Tracking further in #118755.