Skip to content

Commit

Permalink
Don't create temporary and redundant ImmutableArray<string>
Browse files Browse the repository at this point in the history
  • Loading branch information
@ladipro
ladipro committed Feb 16, 2021
1 parent 4510f5a commit 94e3c19
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 47 deletions.
6 changes: 3 additions & 3 deletions src/Build/Evaluation/Context/EvaluationContext.cs
View file
Expand Up @@ -3,7 +3,7 @@

using System;
using System.Collections.Concurrent;
using System.Collections.Immutable;
using System.Collections.Generic;
using System.Threading;
using Microsoft.Build.BackEnd.SdkResolution;
using Microsoft.Build.FileSystem;
Expand Down Expand Up @@ -48,7 +48,7 @@ public enum SharingPolicy
/// <summary>
/// Key to file entry list. Example usages: cache glob expansion and intermediary directory expansions during glob expansion.
/// </summary>
private ConcurrentDictionary<string, ImmutableArray<string>> FileEntryExpansionCache { get; }
private ConcurrentDictionary<string, IReadOnlyList<string>> FileEntryExpansionCache { get; }

private EvaluationContext(SharingPolicy policy, IFileSystem fileSystem)
{
Expand All @@ -61,7 +61,7 @@ private EvaluationContext(SharingPolicy policy, IFileSystem fileSystem)
Policy = policy;

SdkResolverService = new CachingSdkResolverService();
FileEntryExpansionCache = new ConcurrentDictionary<string, ImmutableArray<string>>();
FileEntryExpansionCache = new ConcurrentDictionary<string, IReadOnlyList<string>>();
FileSystem = fileSystem ?? new CachingFileSystemWrapper(FileSystems.Default);
EngineFileUtilities = new EngineFileUtilities(new FileMatcher(FileSystem, FileEntryExpansionCache));
}
Expand Down
63 changes: 31 additions & 32 deletions src/Shared/FileMatcher.cs
View file
Expand Up @@ -9,7 +9,6 @@
using System.Linq;
using System.Text.RegularExpressions;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading.Tasks;
using Microsoft.Build.Shared.FileSystem;
using Microsoft.Build.Utilities;
Expand Down Expand Up @@ -37,10 +36,10 @@ internal class FileMatcher
internal static readonly char[] directorySeparatorCharacters = FileUtilities.Slashes;

// until Cloudbuild switches to EvaluationContext, we need to keep their dependence on global glob caching via an environment variable
private static readonly Lazy<ConcurrentDictionary<string, ImmutableArray<string>>> s_cachedGlobExpansions = new Lazy<ConcurrentDictionary<string, ImmutableArray<string>>>(() => new ConcurrentDictionary<string, ImmutableArray<string>>(StringComparer.OrdinalIgnoreCase));
private static readonly Lazy<ConcurrentDictionary<string, IReadOnlyList<string>>> s_cachedGlobExpansions = new Lazy<ConcurrentDictionary<string, IReadOnlyList<string>>>(() => new ConcurrentDictionary<string, IReadOnlyList<string>>(StringComparer.OrdinalIgnoreCase));
private static readonly Lazy<ConcurrentDictionary<string, object>> s_cachedGlobExpansionsLock = new Lazy<ConcurrentDictionary<string, object>>(() => new ConcurrentDictionary<string, object>(StringComparer.OrdinalIgnoreCase));

private readonly ConcurrentDictionary<string, ImmutableArray<string>> _cachedGlobExpansions;
private readonly ConcurrentDictionary<string, IReadOnlyList<string>> _cachedGlobExpansions;
private readonly Lazy<ConcurrentDictionary<string, object>> _cachedGlobExpansionsLock = new Lazy<ConcurrentDictionary<string, object>>(() => new ConcurrentDictionary<string, object>(StringComparer.OrdinalIgnoreCase));

/// <summary>
Expand Down Expand Up @@ -82,20 +81,20 @@ private static class FileSpecRegexParts
/// </summary>
public static FileMatcher Default = new FileMatcher(FileSystems.Default, null);

public FileMatcher(IFileSystem fileSystem, ConcurrentDictionary<string, ImmutableArray<string>> fileEntryExpansionCache = null) : this(
public FileMatcher(IFileSystem fileSystem, ConcurrentDictionary<string, IReadOnlyList<string>> fileEntryExpansionCache = null) : this(
fileSystem,
(entityType, path, pattern, projectDirectory, stripProjectDirectory) => GetAccessibleFileSystemEntries(
fileSystem,
entityType,
path,
pattern,
projectDirectory,
stripProjectDirectory),
stripProjectDirectory).ToArray(),
fileEntryExpansionCache)
{
}

public FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEntries, ConcurrentDictionary<string, ImmutableArray<string>> getFileSystemDirectoryEntriesCache = null)
internal FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEntries, ConcurrentDictionary<string, IReadOnlyList<string>> getFileSystemDirectoryEntriesCache = null)
{
if (Traits.Instance.MSBuildCacheFileEnumerations)
{
Expand Down Expand Up @@ -123,7 +122,7 @@ public FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEnt
path,
pattern,
directory,
projectDirectory));
projectDirectory).ToArray());
}
return getFileSystemEntries(type, path, pattern, directory, projectDirectory);
};
Expand All @@ -148,8 +147,8 @@ internal enum FileSystemEntity
/// <param name="pattern">The file pattern.</param>
/// <param name="projectDirectory"></param>
/// <param name="stripProjectDirectory"></param>
/// <returns>An immutable array of filesystem entries.</returns>
internal delegate ImmutableArray<string> GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory);
/// <returns>An enumerable of filesystem entries.</returns>
internal delegate IEnumerable<string> GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory);

internal static void ClearFileEnumerationsCache()
{
Expand Down Expand Up @@ -208,7 +207,7 @@ internal static bool HasPropertyOrItemReferences(string filespec)
/// <param name="stripProjectDirectory">If true the project directory should be stripped</param>
/// <param name="fileSystem">The file system abstraction to use that implements file system operations</param>
/// <returns></returns>
private static ImmutableArray<string> GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
private static IEnumerable<string> GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
{
path = FileUtilities.FixFilePath(path);
switch (entityType)
Expand All @@ -220,18 +219,18 @@ private static ImmutableArray<string> GetAccessibleFileSystemEntries(IFileSystem
ErrorUtilities.VerifyThrow(false, "Unexpected filesystem entity type.");
break;
}
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}

/// <summary>
/// Returns an immutable array of file system entries matching the specified search criteria. Inaccessible or non-existent file
/// Returns an enumerable of file system entries matching the specified search criteria. Inaccessible or non-existent file
/// system entries are skipped.
/// </summary>
/// <param name="path"></param>
/// <param name="pattern"></param>
/// <param name="fileSystem">The file system abstraction to use that implements file system operations</param>
/// <returns>An immutable array of matching file system entries (can be empty).</returns>
private static ImmutableArray<string> GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern)
/// <returns>An enumerable of matching file system entries (can be empty).</returns>
private static IEnumerable<string> GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern)
{
if (fileSystem.DirectoryExists(path))
{
Expand All @@ -241,7 +240,7 @@ private static ImmutableArray<string> GetAccessibleFilesAndDirectories(IFileSyst
? fileSystem.EnumerateFileSystemEntries(path, pattern)
.Where(o => IsMatch(Path.GetFileName(o), pattern))
: fileSystem.EnumerateFileSystemEntries(path, pattern)
).ToImmutableArray();
);
}
// for OS security
catch (UnauthorizedAccessException)
Expand All @@ -255,7 +254,7 @@ private static ImmutableArray<string> GetAccessibleFilesAndDirectories(IFileSyst
}
}

return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}

/// <summary>
Expand Down Expand Up @@ -297,7 +296,7 @@ private static bool ShouldEnforceMatching(string searchPattern)
/// <param name="stripProjectDirectory"></param>
/// <param name="fileSystem">The file system abstraction to use that implements file system operations</param>
/// <returns>Files that can be accessed.</returns>
private static ImmutableArray<string> GetAccessibleFiles
private static IEnumerable<string> GetAccessibleFiles
(
IFileSystem fileSystem,
string path,
Expand Down Expand Up @@ -341,17 +340,17 @@ bool stripProjectDirectory
files = RemoveInitialDotSlash(files);
}

return files.ToImmutableArray();
return files;
}
catch (System.Security.SecurityException)
{
// For code access security.
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}
catch (System.UnauthorizedAccessException)
{
// For OS security.
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}
}

Expand All @@ -365,7 +364,7 @@ bool stripProjectDirectory
/// <param name="pattern">Pattern to match</param>
/// <param name="fileSystem">The file system abstraction to use that implements file system operations</param>
/// <returns>Accessible directories.</returns>
private static ImmutableArray<string> GetAccessibleDirectories
private static IEnumerable<string> GetAccessibleDirectories
(
IFileSystem fileSystem,
string path,
Expand Down Expand Up @@ -399,17 +398,17 @@ string pattern
directories = RemoveInitialDotSlash(directories);
}

return directories.ToImmutableArray();
return directories;
}
catch (System.Security.SecurityException)
{
// For code access security.
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}
catch (System.UnauthorizedAccessException)
{
// For OS security.
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}
}

Expand Down Expand Up @@ -500,9 +499,10 @@ GetFileSystemEntries getFileSystemEntries
else
{
// getFileSystemEntries(...) returns an empty array if longPath doesn't exist.
ImmutableArray<string> entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false);
IEnumerable<string> entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false);

if (0 == entries.Length)
int entriesCount = entries.Count();
if (0 == entriesCount)
{
// The next part doesn't exist. Therefore, no more of the path will exist.
// Just return the rest.
Expand All @@ -514,12 +514,12 @@ GetFileSystemEntries getFileSystemEntries
}

// Since we know there are no wild cards, this should be length one.
ErrorUtilities.VerifyThrow(entries.Length == 1,
ErrorUtilities.VerifyThrow(entriesCount == 1,
"Unexpected number of entries ({3}) found when enumerating '{0}' under '{1}'. Original path was '{2}'",
parts[i], longPath, path, entries.Length);
parts[i], longPath, path, entriesCount);

// Entries[0] contains the full path.
longPath = entries[0];
longPath = entries.First();

// We just want the trailing node.
longParts[i - startingElement] = Path.GetFileName(longPath);
Expand Down Expand Up @@ -1898,7 +1898,7 @@ internal string[] GetFiles

var enumerationKey = ComputeFileEnumerationCacheKey(projectDirectoryUnescaped, filespecUnescaped, excludeSpecsUnescaped);

ImmutableArray<string> files;
IReadOnlyList<string> files;
if (!_cachedGlobExpansions.TryGetValue(enumerationKey, out files))
{
// avoid parallel evaluations of the same wildcard by using a unique lock for each wildcard
Expand All @@ -1914,8 +1914,7 @@ internal string[] GetFiles
GetFilesImplementation(
projectDirectoryUnescaped,
filespecUnescaped,
excludeSpecsUnescaped)
.ToImmutableArray());
excludeSpecsUnescaped));
}
}
}
Expand Down
23 changes: 11 additions & 12 deletions src/Shared/UnitTests/FileMatcher_Tests.cs
View file
Expand Up @@ -5,7 +5,6 @@
using Shouldly;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -504,54 +503,54 @@ public void WildcardMatching()
* Simulate Directories.GetFileSystemEntries where file names are short.
*
*/
private static ImmutableArray<string> GetFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
private static IReadOnlyList<string> GetFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
{
if
(
pattern == @"LONGDI~1"
&& (@"D:\" == path || @"\\server\share\" == path || path.Length == 0)
)
{
return ImmutableArray.Create(Path.Combine(path, "LongDirectoryName"));
return new string[] { Path.Combine(path, "LongDirectoryName") };
}
else if
(
pattern == @"LONGSU~1"
&& (@"D:\LongDirectoryName" == path || @"\\server\share\LongDirectoryName" == path || @"LongDirectoryName" == path)
)
{
return ImmutableArray.Create(Path.Combine(path, "LongSubDirectory"));
return new string[] { Path.Combine(path, "LongSubDirectory") };
}
else if
(
pattern == @"LONGFI~1.TXT"
&& (@"D:\LongDirectoryName\LongSubDirectory" == path || @"\\server\share\LongDirectoryName\LongSubDirectory" == path || @"LongDirectoryName\LongSubDirectory" == path)
)
{
return ImmutableArray.Create(Path.Combine(path, "LongFileName.txt"));
return new string[] { Path.Combine(path, "LongFileName.txt") };
}
else if
(
pattern == @"pomegr~1"
&& @"c:\apple\banana\tomato" == path
)
{
return ImmutableArray.Create(Path.Combine(path, "pomegranate"));
return new string[] { Path.Combine(path, "pomegranate") };
}
else if
(
@"c:\apple\banana\tomato\pomegranate\orange" == path
)
{
// No files exist here. This is an empty directory.
return ImmutableArray<string>.Empty;
return Array.Empty<string>();
}
else
{
Console.WriteLine("GetFileSystemEntries('{0}', '{1}')", path, pattern);
Assert.True(false, "Unexpected input into GetFileSystemEntries");
}
return ImmutableArray.Create("<undefined>");
return new string[] { "<undefined>" };
}

private static readonly char S = Path.DirectorySeparatorChar;
Expand Down Expand Up @@ -2083,7 +2082,7 @@ private void GetMatchingDirectories(string[] candidates, string path, string pat
/// <param name="path">The path to search.</param>
/// <param name="pattern">The pattern to search (may be null)</param>
/// <returns>The matched files or folders.</returns>
internal ImmutableArray<string> GetAccessibleFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
internal IReadOnlyList<string> GetAccessibleFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
{
string normalizedPath = Normalize(path);

Expand All @@ -2102,7 +2101,7 @@ internal ImmutableArray<string> GetAccessibleFileSystemEntries(FileMatcher.FileS
GetMatchingDirectories(_fileSet3, normalizedPath, pattern, files);
}

return files.ToImmutableArray();
return files.ToList();
}

/// <summary>
Expand Down Expand Up @@ -2355,9 +2354,9 @@ private static void MatchDriver(string filespec, string[] excludeFilespecs, stri
/// <param name="path"></param>
/// <param name="pattern"></param>
/// <returns>Array of matching file system entries (can be empty).</returns>
private static ImmutableArray<string> GetFileSystemEntriesLoopBack(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
private static IReadOnlyList<string> GetFileSystemEntriesLoopBack(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory)
{
return ImmutableArray.Create(Path.Combine(path, pattern));
return new string[] { Path.Combine(path, pattern) };
}

/*************************************************************************************
Expand Down

0 comments on commit 94e3c19

Please sign in to comment.