GeezerVison

Open questions on this are:

• does it work with the new Windows API (I'll test when I get access to a build with it)
• can we change st_ctime immediately or is it better to just not expose ChangeTime and use both st_ctime and st_birthtime for the same value?

I decided that we can't just change st_ctime, so it's deprecated instead (in docs) but still returns the same as st_birthtime for now.

This supersedes #99755, or at least will conflict so much that it will need to be reimplemented.

@eryksun Could you take a look at this for me? Also, the API is slightly different from the last time you looked - we got some changes made in the OS to suit our needs better.

@zooba , I was analyzing some data from these changes, and I noticed that we have some "slow" calls from isdir/isfile/exists that are no longer calling stat due to #101324 (gh-101196: Make isdir/isfile/exists faster on Windows) by @mdboom . I suspect that those functions could be made faster by using the new and improved stat, when it exists. Not exists() might be a wash, but it's probably worth checking and running those benchmarks from #101324 again.

They shouldn't really be able to be any faster. GetFileAttributesW was already using the fast path that we now have for more info with the new API, so in theory they should be slightly faster due to a little less memcpy.

But since you've confirmed you're running these changes and haven't complained, I'll take that as the best signal I'm going to get that they're good and merge it in (tomorrow, just in case)!

They shouldn't really be able to be any faster. GetFileAttributesW was already using the fast path that we now have for more info with the new API, so in theory they should be slightly faster due to a little less memcpy.

But since you've confirmed you're running these changes and haven't complained, I'll take that as the best signal I'm going to get that they're good and merge it in (tomorrow, just in case)!

Thanks for replying, but I'm not sure I understand. The new isdir/isfile/islink are no longer calling os_stat, but are calling CreateFile + GetFileInformationByHandleEx + CloseHandle, which is the "slow" pattern that this PR was replacing with GetFileInformationByName for better perf. So with this change, stat is faster, but isdir/isfile/islink are still running the slower code.

What am I missing here?

Huh, you're right. You're missing that I misremembered how we implemented those.

They should probably change to GetFileAttributes and check for a reparse point that way, then use the slow path for symlinks (except for the case where we only want the symlink).

In any case, now this is merged, it can be a new issue.

@zooba, I was waiting to review this PR until GetFileInformationByName() is pushed to the general availability channel. I'm surprised that you merged code that depends on a new Windows API function that isn't generally available and has no documentation -- not even a blog post. CPython has never been that aggressive in adopting new Windows features.

They shouldn't really be able to be any faster. GetFileAttributesW was already using the fast path that we now have for more info with the new API, so in theory they should be slightly faster due to a little less memcpy.

The new ntpath.is*() functions (e.g. os__path_isdir_impl) use CreateFileW() and GetFileInformationByHandleEx(). Using GetFileAttributesW() would still require a fallback implementation that calls CreateFileW() for reparse points. It was simpler to implement just the CreateFileW() path. Also, testing showed that using CreateFileW() on balance was about as fast or faster than GetFileAttributesW(). For reparse points, using just CreateFileW() was obviously faster, since we have to call CreateFileW() anyway. Here's what I said at the time:

For me, this implementation takes about about a third less time than using os.stat(). It takes about 10% more time than using GetFileAttributesW(), but using GetFileAttributesW() is significantly more expensive for a reparse point (e.g. symlink, junction) because CreateFileW() has to be called to traverse it, which means the file is opened and queried twice.

On the other hand, at the time, my tests showed that NtQueryInformationByName() was significantly faster than NtQueryAttributesFile(). NtQueryInformationByName() uses a completely different implementation in the kernel. In principle, NtQueryAttributesFile() could take the same path, so maybe they've updated its implementation in the development channel. Either way, the ntpath.is*() functions would benefit from using this faster path to the filesystem information.

I'm surprised that you merged code that depends on a new Windows API function that isn't generally available and has no documentation -- not even a blog post. CPython has never been that aggressive in adopting new Windows features.

It's not a dependency - the behaviour is identical regardless of whether the API is present or not - it's just a perf optimisation when the API is available.

And we've always been this quick to adopt Windows features when I've been the one advocating to get them added into Windows for the sake of Python ;) There just haven't been a whole lot of them.

The new ntpath.is*() functions ...

Yes, I acknowledged that I made a mistake with my earlier statement, and there's a new bug to update them and I already have someone looking at it (when they get time - if not, I'll probably get to it myself). Having gone through the functionality with one of the filesystem devs, GetFileAttributesW should be optimal for the non-reparse case, and as it will tell us if it was a reparse point, it's a more efficient first check than GetFileInformationByName (which also has the same fallback). So it doesn't actually depend on this change at all.