Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Directory.CreateDirectory performance #73942

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ namespace System.IO
{
internal static partial class FileSystem
{
public static bool DirectoryExists(string? fullPath)
public static bool DirectoryExists(string fullPath)
{
return DirectoryExists(fullPath, out _);
}

private static bool DirectoryExists(string? path, out int lastError)
private static bool DirectoryExists(string fullPath, out int lastError)
{
Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data = default;
lastError = FillAttributeInfo(path, ref data, returnErrorOnNotFound: true);
lastError = FillAttributeInfo(fullPath, ref data, returnErrorOnNotFound: true);

return
(lastError == 0) &&
Expand All @@ -44,27 +44,27 @@ public static bool FileExists(string fullPath)
/// Returns 0 on success, otherwise a Win32 error code. Note that
/// classes should use -1 as the uninitialized state for dataInitialized.
/// </summary>
/// <param name="path">The file path from which the file attribute information will be filled.</param>
/// <param name="fullPath">The file path from which the file attribute information will be filled.</param>
/// <param name="data">A struct that will contain the attribute information.</param>
/// <param name="returnErrorOnNotFound">Return the error code for not found errors?</param>
internal static int FillAttributeInfo(string? path, ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data, bool returnErrorOnNotFound)
internal static int FillAttributeInfo(string? fullPath, ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA data, bool returnErrorOnNotFound)
{
int errorCode = Interop.Errors.ERROR_SUCCESS;

// Neither GetFileAttributes or FindFirstFile like trailing separators
path = PathInternal.TrimEndingDirectorySeparator(path);
fullPath = PathInternal.TrimEndingDirectorySeparator(fullPath);

using (DisableMediaInsertionPrompt.Create())
{
if (!Interop.Kernel32.GetFileAttributesEx(path, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data))
if (!Interop.Kernel32.GetFileAttributesEx(fullPath, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref data))
{
errorCode = Marshal.GetLastWin32Error();

if (!IsPathUnreachableError(errorCode))
{
// Assert so we can track down other cases (if any) to add to our test suite
Debug.Assert(errorCode == Interop.Errors.ERROR_ACCESS_DENIED || errorCode == Interop.Errors.ERROR_SHARING_VIOLATION || errorCode == Interop.Errors.ERROR_SEM_TIMEOUT,
$"Unexpected error code getting attributes {errorCode} from path {path}");
$"Unexpected error code getting attributes {errorCode} from path {fullPath}");

// Files that are marked for deletion will not let you GetFileAttributes,
// ERROR_ACCESS_DENIED is given back without filling out the data struct.
Expand All @@ -80,7 +80,7 @@ internal static int FillAttributeInfo(string? path, ref Interop.Kernel32.WIN32_F
// cases that we know we don't want to retry on.

Interop.Kernel32.WIN32_FIND_DATA findData = default;
using (SafeFindHandle handle = Interop.Kernel32.FindFirstFile(path!, ref findData))
using (SafeFindHandle handle = Interop.Kernel32.FindFirstFile(fullPath!, ref findData))
{
if (handle.IsInvalid)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.IO;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;

namespace System.IO
Expand All @@ -26,72 +27,86 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr
return;
}

List<string> stackDir = new List<string>();
fixed (byte* pSecurityDescriptor = securityDescriptor)
{
Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
{
nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
lpSecurityDescriptor = (IntPtr)pSecurityDescriptor
};

// Attempt to figure out which directories don't exist, and only
// create the ones we need. Note that FileExists may fail due
// to Win32 ACL's preventing us from seeing a directory, and this
// isn't threadsafe.
// We know the directory doesn't exist. Before doing more work to parse the path into
// subdirectories and checking each one for existence, try to just create the directory.
// If it works, which is a common case when the parent directory already exists, we're done.
// Otherwise, ignore the error and proceed with the full handling. This makes a fast path
// faster and a slow path a little bit slower.
if (Interop.Kernel32.CreateDirectory(fullPath, &secAttrs))
{
return;
}

bool somepathexists = false;
int length = fullPath.Length;
// Use enough stack space to hold three directory paths. After that, ValueListBuilder
// will grow with ArrayPool arrays.
ThreeObjects scratch = default;
var stackDir = new ValueListBuilder<string>(MemoryMarshal.CreateSpan(ref Unsafe.As<object, string>(ref scratch.Arg0!), 3));

// We need to trim the trailing slash or the code will try to create 2 directories of the same name.
if (length >= 2 && PathInternal.EndsInDirectorySeparator(fullPath.AsSpan()))
{
length--;
}
// Attempt to figure out which directories don't exist, and only
// create the ones we need. Note that FileExists may fail due
// to Win32 ACL's preventing us from seeing a directory, and this
// isn't threadsafe.

int lengthRoot = PathInternal.GetRootLength(fullPath.AsSpan());
bool somepathexists = false;
Copy link
Member

Choose a reason for hiding this comment

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

nit: I know it was named like this before your changes

Suggested change
bool somepathexists = false;
bool somePathExists = false;

int length = fullPath.Length;

if (length > lengthRoot)
{
// Special case root (fullpath = X:\\)
int i = length - 1;
while (i >= lengthRoot && !somepathexists)
// We need to trim the trailing slash or the code will try to create 2 directories of the same name.
if (length >= 2 && PathInternal.EndsInDirectorySeparator(fullPath.AsSpan()))
{
string dir = fullPath.Substring(0, i + 1);
length--;
}

if (!DirectoryExists(dir)) // Create only the ones missing
{
stackDir.Add(dir);
}
else
{
somepathexists = true;
}
int lengthRoot = PathInternal.GetRootLength(fullPath.AsSpan());

while (i > lengthRoot && !PathInternal.IsDirectorySeparator(fullPath[i]))
if (length > lengthRoot)
{
// Special case root (fullpath = X:\\)
int i = length - 1;
while (i >= lengthRoot && !somepathexists)
{
string dir = fullPath.Substring(0, i + 1);

if (!DirectoryExists(dir)) // Create only the ones missing
{
stackDir.Append(dir);
}
else
{
somepathexists = true;
}

while (i > lengthRoot && !PathInternal.IsDirectorySeparator(fullPath[i]))
{
i--;
}

i--;
}

i--;
}
}

int count = stackDir.Count;
bool r = true;
int firstError = 0;
string errorString = fullPath;

fixed (byte* pSecurityDescriptor = securityDescriptor)
{
Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
{
nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
lpSecurityDescriptor = (IntPtr)pSecurityDescriptor
};
int count = stackDir.Length;
bool r = true;
int firstError = 0;
string errorString = fullPath;

while (stackDir.Count > 0)
while (stackDir.Length > 0)
{
string name = stackDir[stackDir.Count - 1];
stackDir.RemoveAt(stackDir.Count - 1);
ref string slot = ref stackDir[^1];
string name = slot;
slot = null!; // to avoid keeping these strings alive in pooled arrays
stackDir.Length--;

r = Interop.Kernel32.CreateDirectory(name, &secAttrs);
if (!r && (firstError == 0))
{
int currentError = Marshal.GetLastWin32Error();
// While we tried to avoid creating directories that don't
// exist above, there are at least two cases that will
// cause us to see ERROR_ALREADY_EXISTS here. FileExists
Expand All @@ -100,6 +115,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr
// create the directory between the time we check and the
// time we try using the directory. Thirdly, it could
// fail because the target does exist, but is a file.
int currentError = Marshal.GetLastWin32Error();
if (currentError != Interop.Errors.ERROR_ALREADY_EXISTS)
{
firstError = currentError;
Expand All @@ -115,27 +131,28 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr
}
}
}
}

// We need this check to mask OS differences
// Handle CreateDirectory("X:\\") when X: doesn't exist. Similarly for n/w paths.
if ((count == 0) && !somepathexists)
{
string? root = Path.GetPathRoot(fullPath);
stackDir.Dispose();

if (!DirectoryExists(root))
// We need this check to mask OS differences
// Handle CreateDirectory("X:\\") when X: doesn't exist. Similarly for n/w paths.
if ((count == 0) && !somepathexists)
{
throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, root);
}
string? root = Path.GetPathRoot(fullPath);
if (root is null || !DirectoryExists(root))
{
throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, root);
}

return;
}
return;
}

// Only throw an exception if creating the exact directory we
// wanted failed to work correctly.
if (!r && (firstError != 0))
{
throw Win32Marshal.GetExceptionForWin32Error(firstError, errorString);
// Only throw an exception if creating the exact directory we
// wanted failed to work correctly.
if (!r && (firstError != 0))
{
throw Win32Marshal.GetExceptionForWin32Error(firstError, errorString);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@
Link="Common\System\IO\FileSystem.Attributes.Windows.cs" />
<Compile Include="$(CommonPath)System\IO\FileSystem.DirectoryCreation.Windows.cs"
Link="Common\System\IO\FileSystem.DirectoryCreation.Windows.cs" />
<Compile Include="$(CoreLibSharedDir)System\Collections\Generic\ValueListBuilder.cs"
Link="Common\System\Collections\Generic\ValueListBuilder.cs" />
<Compile Include="$(CoreLibSharedDir)System\ParamsArray.cs"
Link="Common\System\ParamsArray.cs" />
</ItemGroup>
<ItemGroup>
<Reference Include="System.Buffers" Condition="'$(TargetPlatformIdentifier)' == 'windows'" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,31 @@ private void Init(string originalPath, string? fullPath = null, string? fileName
fullPath ??= originalPath;
fullPath = isNormalized ? fullPath : Path.GetFullPath(fullPath);

_name = fileName ?? (PathInternal.IsRoot(fullPath.AsSpan()) ?
fullPath.AsSpan() :
Path.GetFileName(Path.TrimEndingDirectorySeparator(fullPath.AsSpan()))).ToString();
_name = fileName;

FullPath = fullPath;

_isNormalized = isNormalized;
}

public override string Name
{
get
{
string? name = _name;
if (name is null)
{
ReadOnlySpan<char> fullPath = FullPath.AsSpan();
_name = name = (PathInternal.IsRoot(fullPath) ?
fullPath :
Path.GetFileName(Path.TrimEndingDirectorySeparator(fullPath))).ToString();
}

return name;

Copy link
Member

Choose a reason for hiding this comment

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

nit: remove whitespace

Suggested change

}
}

public DirectoryInfo? Parent
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ internal FileInfo(string originalPath, string? fullPath = null, string? fileName
Debug.Assert(!isNormalized || !PathInternal.IsPartiallyQualified(fullPath.AsSpan()), "should be fully qualified if normalized");

FullPath = isNormalized ? fullPath ?? originalPath : Path.GetFullPath(fullPath);
_name = fileName ?? Path.GetFileName(originalPath);
_name = fileName;
}

public override string Name => _name ??= Path.GetFileName(OriginalPath);

public long Length
{
get
Expand Down Expand Up @@ -156,7 +158,7 @@ public void MoveTo(string destFileName, bool overwrite)

FullPath = fullDestFileName;
OriginalPath = destFileName;
_name = Path.GetFileName(fullDestFileName);
_name = null;

// Flush any cached information about the file.
Invalidate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.IO;
using System.Runtime.Serialization;
using System.Runtime.Versioning;
Expand All @@ -14,7 +15,7 @@ public abstract partial class FileSystemInfo : MarshalByRefObject, ISerializable
protected string FullPath = null!; // fully qualified path of the file or directory
protected string OriginalPath = null!; // path passed in by the user

internal string _name = null!; // Fields initiated in derived classes
internal string? _name;

private string? _linkTarget;
private bool _linkTargetIsValid;
Expand Down Expand Up @@ -55,7 +56,14 @@ public string Extension
}
}

public virtual string Name => _name;
public virtual string Name
{
get
{
Debug.Fail("Property is abstract in the ref assembly and both Directory/FileInfo override it.");
return _name!;
}
}

// Whether a file/directory exists
public virtual bool Exists
Expand Down