Skip to content

Reduce allocations in NavigationCmdletProvider.NormalizePath() #10038

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 3 commits into from
Jul 3, 2019
Merged

Reduce allocations in NavigationCmdletProvider.NormalizePath() #10038

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8623ca
Reduce allocations in NormalizePath()
iSazonov Jul 1, 2019
bc9c48f
Remove extra var and use char in EndsWith()
iSazonov Jul 1, 2019
3dc9b20
Remove extra var parentEndsWithForwardSlash
iSazonov Jul 1, 2019
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
39 changes: 27 additions & 12 deletions src/System.Management.Automation/namespaces/NavigationProviderBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,38 +905,53 @@ protected virtual object MoveItemDynamicParameters(
/// those errors.
/// </summary>
/// <param name="path">
/// The path to normalize
/// The path to normalize.
/// </param>
/// <returns>
/// Normalized path or the original path
/// Normalized path or the original path.
/// </returns>
private string NormalizePath(string path)
{
// If we have a mix of slashes, then we may introduce an error by normalizing the path.
// For example: path HKCU:\Test\/ is pointing to a subkey '/' of 'HKCU:\Test', if we
// normalize it, then we will get a wrong path.
bool pathHasForwardSlash = path.IndexOf(StringLiterals.AlternatePathSeparator) != -1;
bool pathHasBackSlash = path.IndexOf(StringLiterals.DefaultPathSeparator) != -1;
bool pathHasMixedSlashes = pathHasForwardSlash && pathHasBackSlash;
bool shouldNormalizePath = true;
//
// Fast return if nothing to normalize.
if (path.IndexOf(StringLiterals.AlternatePathSeparator) == -1)
{
return path;
}

string normalizedPath = path.Replace(StringLiterals.AlternatePathSeparator, StringLiterals.DefaultPathSeparator);
bool pathHasBackSlash = path.IndexOf(StringLiterals.DefaultPathSeparator) != -1;
string normalizedPath;

// There is a mix of slashes & the path is rooted & the path exists without normalization.
// In this case, we might want to skip the normalization to the path.
if (pathHasMixedSlashes && IsAbsolutePath(path) && ItemExists(path))
if (pathHasBackSlash && IsAbsolutePath(path) && ItemExists(path))
{
// 1. The path exists and ends with a forward slash, in this case, it's very possible the ending forward slash
// make sense to the underlying provider, so we skip normalization
// 2. The path exists, but not anymore after normalization, then we skip normalization
bool parentEndsWithForwardSlash = path.EndsWith(StringLiterals.AlternatePathSeparatorString, StringComparison.Ordinal);
if (parentEndsWithForwardSlash || !ItemExists(normalizedPath))
if (path.EndsWith(StringLiterals.AlternatePathSeparator))
{
shouldNormalizePath = false;
return path;
}

normalizedPath = path.Replace(StringLiterals.AlternatePathSeparator, StringLiterals.DefaultPathSeparator);

if (!ItemExists(normalizedPath))
{
return path;
}
else
{
return normalizedPath;
}
}

return shouldNormalizePath ? normalizedPath : path;
normalizedPath = path.Replace(StringLiterals.AlternatePathSeparator, StringLiterals.DefaultPathSeparator);

return normalizedPath;
}

/// <summary>
Expand Down