Skip to content

Conversation

@Tarun047
Copy link
Contributor

Added a new API Path.Exists(...) as per the approved spec described in issue #21678.
This API is effectively equivalent to checking File.Exists(...) combined with Directory.Exists(...)
Using this API we only make one syscall to the kernel, which would have been two calls otherwise.

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-System.IO new-api-needs-documentation labels Jan 26, 2022
@ghost
Copy link

ghost commented Jan 26, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Jan 26, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Added a new API Path.Exists(...) as per the approved spec described in issue #21678.
This API is effectively equivalent to checking File.Exists(...) combined with Directory.Exists(...)
Using this API we only make one syscall to the kernel, which would have been two calls otherwise.

Author: Tarun047
Assignees: -
Labels:

area-System.IO, new-api-needs-documentation, community-contribution

Milestone: -

@dnfadmin
Copy link

dnfadmin commented Jan 26, 2022

CLA assistant check
All CLA requirements met.

@danmoseley
Copy link
Member

thanks, I added some comments.

@Tarun047 Tarun047 requested a review from danmoseley January 27, 2022 14:59
@danmoseley
Copy link
Member

BTW @Tarun047 I'm guessing you're running on Windows and relying on PR validation to run the Unix tests. What many of us do that work on Windows is build and run tests on WSL2 (any distro eg., Ubuntu). It works really well. What I find easiest is to clone to my WSL2 distro exactly as I would on Windows, and set up the remote to my fork as I would on Windows, then push and pull via that to share bits. But some people set up remotes directly between Windows and WSL2. Once you have a clone, you just follow the regular doc in this repo for building and running tests on Linux.

Given you've already created a PR for your branch here, if I wanted to share bits between the two without kicking validation again, I would push from one to a new branch, and pull that branch down to the other. Verify tests pass in both, then push that branch to this PR branch.

@danmoseley
Copy link
Member

(Because relying on PR validation to run the tests for you makes development slow and noisy. We generally use PR validation to ensure that all the configuration matrix is tested, once we've run the key configuration(s) locally.)

@Tarun047
Copy link
Contributor Author

Yes @danmoseley I am using Windows machine for the primary development, I do have a M1 Macbook pro, but haven't found time to setup the build infrastructure on it yet, hopefully will do it soon.
Anyway thanks for the tip, I'd use a separate branches if I wanted to share code between 2 machines from next time.

@danmoseley
Copy link
Member

Cool, yes, I think you'd find it more productive for you and also less time burned on validation (which I think consumes 50-100 machines each push) 😸

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tarun047 big thanks for your contribution!

I've added few comments, mostly style but also about adding triple slash comments.

When adding the triple slash comments you can use some of the existing text from https://docs.microsoft.com/en-us/dotnet/api/system.io.file.exists?view=net-6.0

@adamsitnik adamsitnik added this to the 7.0.0 milestone Jan 28, 2022
@adamsitnik
Copy link
Member

FWIW I've benchmarked the two sys-calls that allow us to check whether given file exists or not and the current implementation of the PR is already using the optimal one. 👍

Method IsFile Exists Mean Error StdDev Ratio RatioSD
GetFileAttributesEx False False 7.629 us 0.1387 us 0.0076 us 1.00 0.00
FindFirstFile False False 20.274 us 1.8564 us 0.1018 us 2.66 0.01
GetFileAttributesEx False True 10.898 us 0.5784 us 0.0317 us 1.00 0.00
FindFirstFile False True 20.971 us 1.5526 us 0.0851 us 1.92 0.01
GetFileAttributesEx True False 7.427 us 1.6298 us 0.0893 us 1.00 0.00
FindFirstFile True False 20.477 us 0.6854 us 0.0376 us 2.76 0.03
GetFileAttributesEx True True 14.269 us 0.1665 us 0.0091 us 1.00 0.00
FindFirstFile True True 20.641 us 0.7453 us 0.0409 us 1.45 0.00
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="BenchmarkDotNet" Version="0.13.1" />
  </ItemGroup>
</Project>
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.Win32.SafeHandles;
using System;
using System.IO;
using System.Runtime.InteropServices;

namespace FileExistsWindowsSyscalls
{
    internal class Program
    {
        static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
    }

    public class Syscalls
    {
        string _path;

        [Params(true, false)]
        public bool IsFile;

        [Params(true, false)]
        public bool Exists;

        [GlobalSetup]
        public void Setup()
        {
            _path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());

            if (Exists)
            {
                if (IsFile) File.Create(_path).Dispose();
                else Directory.CreateDirectory(_path);
            }
        }

        [GlobalCleanup]
        public void Cleanup()
        {
            if (Exists)
            {
                if (IsFile) File.Delete(_path);
                else Directory.Delete(_path);
            }
        }

        [Benchmark(Baseline = true)]
        public bool GetFileAttributesEx()
        {
            WIN32_FILE_ATTRIBUTE_DATA data = default;
            return GetFileAttributesEx(_path, GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data);
        }

        [Benchmark]
        public bool FindFirstFile()
        {
            WIN32_FIND_DATA data = default;
            using SafeFindHandle handle = FindFirstFileEx(_path, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0);
            return !handle.IsInvalid;
        }

        [DllImport("kernel32.dll", EntryPoint = "GetFileAttributesExW", CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)]
        private static extern bool GetFileAttributesEx(string name, GET_FILEEX_INFO_LEVELS fileInfoLevel, ref WIN32_FILE_ATTRIBUTE_DATA lpFileInformation);

        [DllImport("kernel32.dll", EntryPoint = "FindFirstFileExW", CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)]
        private static extern SafeFindHandle FindFirstFileEx(string lpFileName, FINDEX_INFO_LEVELS fInfoLevelId, ref WIN32_FIND_DATA lpFindFileData, FINDEX_SEARCH_OPS fSearchOp, IntPtr lpSearchFilter, int dwAdditionalFlags);
    }

    internal enum GET_FILEEX_INFO_LEVELS : uint
    {
        GetFileExInfoStandard = 0x0u,
        GetFileExMaxInfoLevel = 0x1u,
    }

    internal struct FILE_TIME
    {
        internal uint dwLowDateTime;
        internal uint dwHighDateTime;

        internal FILE_TIME(long fileTime)
        {
            dwLowDateTime = (uint)fileTime;
            dwHighDateTime = (uint)(fileTime >> 32);
        }

        internal long ToTicks() => ((long)dwHighDateTime << 32) + dwLowDateTime;
        internal DateTime ToDateTimeUtc() => DateTime.FromFileTimeUtc(ToTicks());
        internal DateTimeOffset ToDateTimeOffset() => DateTimeOffset.FromFileTime(ToTicks());
    }

    [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
    internal unsafe struct WIN32_FIND_DATA
    {
        internal const int MAX_PATH = 260;

        internal uint dwFileAttributes;
        internal FILE_TIME ftCreationTime;
        internal FILE_TIME ftLastAccessTime;
        internal FILE_TIME ftLastWriteTime;
        internal uint nFileSizeHigh;
        internal uint nFileSizeLow;
        internal uint dwReserved0;
        internal uint dwReserved1;
        private fixed char _cFileName[MAX_PATH];
        private fixed char _cAlternateFileName[14];

        internal ReadOnlySpan<char> cFileName
        {
            get { fixed (char* c = _cFileName) return new ReadOnlySpan<char>(c, MAX_PATH); }
        }
    }

    internal struct WIN32_FILE_ATTRIBUTE_DATA
    {
        internal int dwFileAttributes;
        internal FILE_TIME ftCreationTime;
        internal FILE_TIME ftLastAccessTime;
        internal FILE_TIME ftLastWriteTime;
        internal uint nFileSizeHigh;
        internal uint nFileSizeLow;

        internal void PopulateFrom(ref WIN32_FIND_DATA findData)
        {
            dwFileAttributes = (int)findData.dwFileAttributes;
            ftCreationTime = findData.ftCreationTime;
            ftLastAccessTime = findData.ftLastAccessTime;
            ftLastWriteTime = findData.ftLastWriteTime;
            nFileSizeHigh = findData.nFileSizeHigh;
            nFileSizeLow = findData.nFileSizeLow;
        }
    }

    internal enum FINDEX_INFO_LEVELS : uint
    {
        FindExInfoStandard = 0x0u,
        FindExInfoBasic = 0x1u,
        FindExInfoMaxInfoLevel = 0x2u,
    }

    internal enum FINDEX_SEARCH_OPS : uint
    {
        FindExSearchNameMatch = 0x0u,
        FindExSearchLimitToDirectories = 0x1u,
        FindExSearchLimitToDevices = 0x2u,
        FindExSearchMaxSearchOp = 0x3u,
    }

    internal sealed class SafeFindHandle : SafeHandleZeroOrMinusOneIsInvalid
    {
        public SafeFindHandle() : base(true) { }

        protected override bool ReleaseHandle() => FindClose(handle);

        [DllImport("kernel32.dll", SetLastError = true)]
        internal static extern bool FindClose(IntPtr hFindFile);
    }
}

@adamsitnik adamsitnik self-assigned this Jan 28, 2022
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some suggestions for how we could improve the docs. PTAL.

@Tarun047 Tarun047 requested a review from adamsitnik January 28, 2022 16:41
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good to me! Thank you for your contribution @Tarun047 !

@adamsitnik adamsitnik merged commit c712129 into dotnet:main Jan 31, 2022
@danmoseley
Copy link
Member

@Tarun047 we have more issues marked up for grabs - perhaps there is another that is interesting to you?

@Tarun047
Copy link
Contributor Author

Tarun047 commented Feb 1, 2022

Sure

@ghost ghost locked as resolved and limited conversation to collaborators Mar 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.IO community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants