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.

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

Jump to bottom

Optimizations for FileMatcher.GetFiles #2572

Merged
merged 2 commits into from Oct 10, 2017
Merged

Optimizations for FileMatcher.GetFiles #2572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c3e87c
Mock File System should return paths with OS specific slashes
cdmihai Oct 3, 2017
246a185
Optimized FileMatcher.GetFiles method (#2392)
maca88 Sep 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions build/NuGetPackages/Microsoft.Build.Framework.nuspec
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@
<tags>MSBuild</tags>
<dependencies>
<group targetFramework=".NETStandard1.3">
<dependency id="System.Threading.Tasks.Parallel" version="4.0.1" />
<dependency id="System.Collections" version="4.0.11" />
<dependency id="System.Diagnostics.Debug" version="4.0.11" />
<dependency id="System.Globalization" version="4.0.11" />
Expand Down
1 change: 1 addition & 0 deletions build/NuGetPackages/Microsoft.Build.Tasks.Core.nuspec
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@
<dependency id="System.Text.RegularExpressions" version="4.1.0" />
<dependency id="System.Threading" version="4.0.11" />
<dependency id="System.Threading.Tasks" version="4.0.11" />
<dependency id="System.Threading.Tasks.Parallel" version="4.0.1" />
<dependency id="System.Threading.Thread" version="4.0.0" />
<dependency id="System.Xml.ReaderWriter" version="4.0.11" />
<dependency id="System.Xml.XDocument" version="4.0.11" />
Expand Down
3 changes: 3 additions & 0 deletions build/NuGetPackages/Microsoft.Build.Utilities.Core.nuspec
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@
<dependency id="System.AppContext" version="4.1.0" />
<dependency id="System.Collections" version="4.0.11" />
<dependency id="System.Collections.Concurrent" version="4.0.12" />
<dependency id="System.Collections.Immutable" version="1.2.0" />
<dependency id="System.Collections.NonGeneric" version="4.0.1" />
<dependency id="System.Console" version="4.0.0" />
<dependency id="System.Diagnostics.Debug" version="4.0.11" />
Expand All @@ -47,13 +48,15 @@
<dependency id="System.Text.RegularExpressions" version="4.1.0" />
<dependency id="System.Threading" version="4.0.11" />
<dependency id="System.Threading.Tasks" version="4.0.11" />
<dependency id="System.Threading.Tasks.Parallel" version="4.0.1" />
<dependency id="System.Threading.Thread" version="4.0.0" />
<dependency id="System.Threading.Timer" version="4.0.1" />
<dependency id="System.Xml.ReaderWriter" version="4.0.11" />
<dependency id="System.Xml.XmlDocument" version="4.0.1" />
</group>
<group targetFramework=".NETFramework4.6">
<dependency id="Microsoft.Build.Framework" version="[$version$]" />
<dependency id="System.Collections.Immutable" version="1.3.1" />
<dependency id="System.Runtime.InteropServices.RuntimeInformation" version="4.3.0" />
</group>
</dependencies>
Expand Down
1 change: 1 addition & 0 deletions build/NuGetPackages/Microsoft.Build.nuspec
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@
<dependency id="System.Threading" version="4.0.11" />
<dependency id="System.Threading.Tasks" version="4.0.11" />
<dependency id="System.Threading.Tasks.Dataflow" version="4.6.0" />
<dependency id="System.Threading.Tasks.Parallel" version="4.0.1" />
<dependency id="System.Threading.Thread" version="4.0.0" />
<dependency id="System.Threading.ThreadPool" version="4.0.10" />
<dependency id="System.Xml.ReaderWriter" version="4.0.11" />
Expand Down
1 change: 1 addition & 0 deletions src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj
View file
Open in desktop
Expand Up @@ -249,6 +249,7 @@
</ProjectReference>
</ItemGroup>
<ItemGroup Condition="'$(NetCoreBuild)' != 'true'">
<Reference Include="System.Collections.Immutable, Version=1.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've had to add this reference in order to build the project. Does anyone know why?

Copy link
Contributor

@cdmihai cdmihai Oct 6, 2017 •
edited

Choose a reason for hiding this comment

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

I think it's because ImmutableArray got exposed to the tests (FileMatcher_Tests, via the new cache). And then FileMatcher_Tests is included in two test dlls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that the two test dlls need the dependency, what I don't understand is that adding the dependency to the project.json don't work for Engine.UnitTests, but it works for the Tasks.UnitTests project. Its seems like that one of the Engine.UnitTests dependency needs a lower version of the Immutable dll, but from the logs it is not clear which one.

<Reference Include="System" />
<Reference Include="System.Configuration" />
<Reference Include="System.Xml" />
Expand Down
2 changes: 2 additions & 0 deletions src/Build.UnitTests/project.json
View file
Open in desktop
Expand Up @@ -10,13 +10,15 @@
"frameworks": {
"net46": {
"Microsoft.Net.Compilers": "2.3.1",
"System.Collections.Immutable": "1.3.1",
"System.Threading.Tasks.Dataflow": "4.5.24.0"
},
"netstandard1.3": {
"dependencies": {
"System.Net.NetworkInformation": "4.1.0",
"Microsoft.NETCore": "5.0.1",
"Microsoft.NETCore.Portable.Compatibility": "1.0.1",
"System.Collections.Immutable": "1.2.0",
"System.Collections.NonGeneric": "4.0.1",
"System.Console": "4.0.0",
"System.Diagnostics.Contracts": "4.0.1",
Expand Down
3 changes: 2 additions & 1 deletion src/Build/Evaluation/LazyItemEvaluator.IncludeOperation.cs
View file
Open in desktop
Expand Up @@ -99,7 +99,8 @@ protected override ICollection<I> SelectItems(ImmutableList<ItemData>.Builder li
string[] includeSplitFilesEscaped = EngineFileUtilities.GetFileListEscaped(
_rootDirectory,
glob,
excludePatternsForGlobs
excludePatternsForGlobs,
entriesCache: EntriesCache
);

// itemsToAdd might grow 0 or more times during the following iteration. Proactively increase its capacity to ensure only one growth happens
Expand Down
6 changes: 6 additions & 0 deletions src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs
View file
Open in desktop
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand Down Expand Up @@ -44,6 +45,11 @@ protected LazyItemOperation(OperationBuilder builder, LazyItemEvaluator<P, I, M,
_itemSpec.Expander = _expander;
}

/// <summary>
/// Cache used for caching IO operation results
/// </summary>
protected ConcurrentDictionary<string, ImmutableArray<string>> EntriesCache => _lazyEvaluator._entriesCache;

public virtual void Apply(ImmutableList<ItemData>.Builder listBuilder, ImmutableHashSet<string> globsToIgnore)
{
var items = SelectItems(listBuilder, globsToIgnore).ToList();
Expand Down
6 changes: 6 additions & 0 deletions src/Build/Evaluation/LazyItemEvaluator.cs
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand Down Expand Up @@ -45,6 +46,11 @@ internal partial class LazyItemEvaluator<P, I, M, D>
new Dictionary<string, LazyItemList>() :
new Dictionary<string, LazyItemList>(StringComparer.OrdinalIgnoreCase);

/// <summary>
/// Cache used for caching IO operation results
/// </summary>
private readonly ConcurrentDictionary<string, ImmutableArray<string>> _entriesCache = new ConcurrentDictionary<string, ImmutableArray<string>>();

public LazyItemEvaluator(IEvaluatorData<P, I, M, D> data, IItemFactory<I, I> itemFactory, LoggingContext loggingContext)
{
_outerEvaluatorData = data;
Expand Down
13 changes: 9 additions & 4 deletions src/Build/Utilities/EngineFileUtilities.cs
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Text;
using System.IO;
Expand Down Expand Up @@ -74,16 +75,18 @@ internal static string[] GetFileListUnescaped
/// <param name="filespecEscaped">The filespec to evaluate, escaped.</param>
/// <param name="excludeSpecsEscaped">Filespecs to exclude, escaped.</param>
/// <param name="forceEvaluate">Whether to force file glob expansion when eager expansion is turned off</param>
/// <param name="entriesCache">Cache used for caching IO operation results</param>
/// <returns>Array of file paths, escaped.</returns>
internal static string[] GetFileListEscaped
(
string directoryEscaped,
string filespecEscaped,
IEnumerable<string> excludeSpecsEscaped = null,
bool forceEvaluate = false
bool forceEvaluate = false,
ConcurrentDictionary<string, ImmutableArray<string>> entriesCache = null
)
{
return GetFileList(directoryEscaped, filespecEscaped, true /* returnEscaped */, forceEvaluate, excludeSpecsEscaped);
return GetFileList(directoryEscaped, filespecEscaped, true /* returnEscaped */, forceEvaluate, excludeSpecsEscaped, entriesCache);
}

private static bool FilespecHasWildcards(string filespecEscaped)
Expand Down Expand Up @@ -125,14 +128,16 @@ private static bool FilespecHasWildcards(string filespecEscaped)
/// <param name="returnEscaped"><code>true</code> to return escaped specs.</param>
/// <param name="forceEvaluateWildCards">Whether to force file glob expansion when eager expansion is turned off</param>
/// <param name="excludeSpecsEscaped">The exclude specification, escaped.</param>
/// <param name="entriesCache">Cache used for caching IO operation results</param>
/// <returns>Array of file paths.</returns>
private static string[] GetFileList
(
string directoryEscaped,
string filespecEscaped,
bool returnEscaped,
bool forceEvaluateWildCards,
IEnumerable<string> excludeSpecsEscaped = null
IEnumerable<string> excludeSpecsEscaped = null,
ConcurrentDictionary<string, ImmutableArray<string>> entriesCache = null
)
{
ErrorUtilities.VerifyThrowInternalLength(filespecEscaped, "filespecEscaped");
Expand Down Expand Up @@ -167,7 +172,7 @@ private static string[] GetFileList
// as a relative path, we will get back a bunch of relative paths.
// If the filespec started out as an absolute path, we will get
// back a bunch of absolute paths.
fileList = FileMatcher.GetFiles(directoryUnescaped, filespecUnescaped, excludeSpecsUnescaped);
fileList = FileMatcher.GetFiles(directoryUnescaped, filespecUnescaped, excludeSpecsUnescaped, entriesCache);

ErrorUtilities.VerifyThrow(fileList != null, "We must have a list of files here, even if it's empty.");

Expand Down
1 change: 1 addition & 0 deletions src/Build/project.json
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@
"System.Runtime.Serialization.Primitives": "4.1.1",
"System.Text.Encoding.CodePages" : "4.0.1",
"System.Threading.Tasks.Dataflow": "4.6.0.0",
"System.Threading.Tasks.Parallel": "4.0.1",
"System.Threading.Thread": "4.0.0",
"System.Threading.ThreadPool": "4.0.10",
"System.Xml.XmlDocument": "4.0.1",
Expand Down
1 change: 1 addition & 0 deletions src/Framework/project.json
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
"SourceLink.Create.CommandLine": "2.1.2",
"System.Runtime.Serialization.Primitives": "4.1.1",
"System.Text.Encoding.CodePages" : "4.0.1",
"System.Threading.Tasks.Parallel": "4.0.1",
"System.Threading.Thread": "4.0.0"
}
}
Expand Down