JDK-8259873: (fs) Add java.nio.file.attribute.BasicWithKeyFileAttributeView interface
Renaud Paquay
rpaquay at google.com
Thu Jan 21 18:20:06 UTC 2021
>
>> The recent thread with subject line "Files.isSameFile(Path,Path)" may be
>> useful to re-read. The JDK has been using the volume serial number and
>> file indexes but they are problematic on some volumes (not guarantee to
>> be unique, maybe be zero). The Microsoft folks here have been looking to
>> replace the usage with the "full path" (the approximate equivalent to
>> realpath).
>>
>
Reading the isSameFile thread (starting
https://mail.openjdk.java.net/pipermail/nio-dev/2020-October/007682.html),
it looks like the issue is that some file systems (webdav) return all
zeros. The PR we submitted already takes this case in account, and does not
expose a file key if the volume serial is zero (
https://github.com/openjdk/jdk/pull/2122/files#diff-efc9760571de4e32620d825eddb664e654876be4e2fc5f7ef2ead5461adbf7a0R489).
This ensures a bogus file key is never exposed. Of course, this will void
the performance improvements of the CL (avoiding calls to isSameFile on
such file systems), but I believe the behavior is ok, as I doubt we will
find way to make file keys more efficient than using vol serial number and
file indices, and they are available (and reliable) on the vast majority of
windows systems (ntfs file systems). wdyt?
>
>> We did a lot work in JDK 7 to make the default case (not following sym
>> links) be efficient on Windows. This is the reason for the internal
>> BasicFileAttribuetHolder that FileTreeWalker uses to avoid needing to
>> read the file attributes. Maybe we could lean on that internal interface
>> and try to find options there before adding to the standard API.
>>
>
The first change (NtQueryDirectoryFile) does provide
BasicFileAttributeHolder via WindowsDirectoryStream, which is used by
FileTreeWalker (see the acceptEntry method of WindowsDirectoryStream,
https://github.com/openjdk/jdk/pull/2122/files#diff-447e8d9c0cfa24aa5bc1d30cbfa0843cca93366c79d0e7bfa377c14b656f39c6L147
).
The remaining problem is that FileTreeWalker does *not* have a
BasicFileAttributeHolder for the initial directory, hence does not have a
file key, hence requires calling the slower isSameFile methods. I am open
to suggestions on how to provide a file key for the initial directory other
than exposing a new interface, but afaik, BasicFileAttributeHolder is not
involved in this special case of the initial directory. Essentially, we
need a way for FileTreeWalker to say "I would like attributes including a
file key if possible, even if it means using a slower implementation".
>
>> Switching from FindFirstFile/FindNextFile to NtQueryDirectoryInformation
>> is definitely worth exploring, it will just take significant time to
>> review and will likely go through a few review iterations.
>>
>
Ok, I am not sure I fully understand what this means. We are of course open
to making tweaks to the CL as needed, but do you need anything from us to
move forward at this point? For example, we have a small benchmark app if
needed. Also, for context, we applied these changes to Android Studio about
1.5 year ago (openjdk 8 and 11), and we have not seen issues. (Android
Studio has hundreds of thousands of active Windows users)
>
> How about inviting Microsoft folk to review? They should be experts.
>
That would be great :)
Thanks,
Renaud
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20210121/dfead69d/attachment.htm>
More information about the nio-dev
mailing list