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

Reduce allocations in SolutionParser by 80-90% and cut CPU time 40-70% #8852

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
481c759
Annotate ProjectFileErrorUtilities
drewnoakes Jun 5, 2023
de89fc1
Reduce allocations during SolutionFile parsing
drewnoakes Jun 5, 2023
b0c2fbc
Move TrimQuotes to local function
drewnoakes Jun 5, 2023
276df9c
Reduce allocations due to error checking
drewnoakes Jun 5, 2023
4fe936b
Add tests for GetSolutionFileAndVisualStudioMajorVersions
drewnoakes Jun 5, 2023
eb1d51a
Fix bug in GetSolutionFileAndVisualStudioMajorVersions
drewnoakes Jun 5, 2023
84159d8
Improve regex performance
drewnoakes Jun 5, 2023
b18c261
Code formatting
drewnoakes Jun 5, 2023
6a299cf
Use explicit string comparer
drewnoakes Jun 5, 2023
b88d066
Use pattern matching
drewnoakes Jun 5, 2023
9ad53f7
Cache Path.GetInvalidPathChars()
drewnoakes Jun 5, 2023
8f8baae
Simplify expression
drewnoakes Jun 5, 2023
317cffa
Fix typo
drewnoakes Jun 5, 2023
b608df4
Make field readonly
drewnoakes Jun 5, 2023
00cd85a
Move field
drewnoakes Jun 5, 2023
ec8ba42
Update API docs
drewnoakes Jun 5, 2023
b676da9
Additional unit test case
drewnoakes Jun 5, 2023
2115b49
Reduce file IO buffer size in GetSolutionFileAndVisualStudioMajorVers…
drewnoakes Jun 5, 2023
b02f92e
More use of Span throughout SolutionFile parsing
drewnoakes Jun 5, 2023
6f8be77
Formatting
drewnoakes Jun 5, 2023
3fe34c3
Still more use of Span in SolutionFile parsing
drewnoakes Jun 5, 2023
4ac15a5
Formatting
drewnoakes Jun 5, 2023
4b3ef7b
Reduce string allocations when parsing solution
drewnoakes Jun 7, 2023
620c220
Null annotate SolutionFile and friends
drewnoakes Jun 7, 2023
1f8f14a
Pool more common strings to reduce allocations and retained memory
drewnoakes Jun 7, 2023
7a272fe
Reduce diagnostic noise in IDE
drewnoakes Jun 7, 2023
3c1f989
Update documentation
drewnoakes Jun 7, 2023
7e19e67
Null annotate tests and use raw strings for test data
drewnoakes Jun 7, 2023
c505d2a
Unit test organisation
drewnoakes Jun 7, 2023
90e47d1
Reduce work in ProjectInSolution.CleanseProjectName
drewnoakes Jun 7, 2023
1f963e6
Simplify taking a sliced span from a string
drewnoakes Jun 7, 2023
5eb5c18
Pool "config|platform" strings
drewnoakes Jun 7, 2023
88dcb0b
Compute FullName properties lazily
drewnoakes Jun 7, 2023
0ebd4bb
Span.IndexOf is twice as fast as Array.IndexOf
drewnoakes Jun 7, 2023
d9ee47b
Pool strings to avoid N*M allocations
drewnoakes Jun 7, 2023
e26cb88
Make allocation of AspNetConfigurations lazy
drewnoakes Jun 7, 2023
a260e37
Make allocation of ProjectReferences lazy
drewnoakes Jun 7, 2023
c9e5532
Inline constant
drewnoakes Jun 8, 2023
f4710ea
Clarify comment
drewnoakes Jun 8, 2023
80c0762
Add extension methods to simplify using spans on .NET Framework
drewnoakes Jun 8, 2023
21dd4e8
Move `StringPool` and `StreamLineSpanReader` to `Microsoft.NET.String…
drewnoakes Jun 8, 2023
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
30 changes: 30 additions & 0 deletions src/Build.UnitTests/Construction/ProjectInSolution_Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

namespace Microsoft.Build.Construction;

public sealed class ProjectInSolution_Tests
{
[Theory]
[InlineData("", "")]
[InlineData("Hello", "Hello")]
[InlineData("Hello.world", "Hello_world")]
[InlineData("Hello_world", "Hello_world")]
[InlineData("Hello (world)", "Hello _world_")]
[InlineData("It's 99.9% bug free", "It_s 99_9_ bug free")]
[InlineData("%$@;.()'", "________")]
public void CleanseProjectName(string input, string expected)
{
// Disallowed characters are: %$@;.()'
string actual = ProjectInSolution.CleanseProjectName(input);

Assert.Equal(expected, actual);

if (input == expected)
{
Assert.Same(input, actual);
}
}
}
1,865 changes: 1,107 additions & 758 deletions src/Build.UnitTests/Construction/SolutionFile_Tests.cs

Large diffs are not rendered by default.

5 changes: 1 addition & 4 deletions src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<PackageReference Include="Shouldly" />
<PackageReference Include="System.Net.Http" />
<PackageReference Include="Microsoft.CodeAnalysis.Build.Tasks" />
<PackageReference Include="NuGet.Frameworks" >
<PackageReference Include="NuGet.Frameworks">
<PrivateAssets>all</PrivateAssets>
</PackageReference>

Expand Down Expand Up @@ -80,9 +80,6 @@
<Compile Include="..\UnitTests.Shared\RunnerUtilities.cs">
<ExcludeFromStyleCop>true</ExcludeFromStyleCop>
</Compile>
<Compile Include="..\Shared\UnitTests\StreamHelpers.cs">
<ExcludeFromStyleCop>true</ExcludeFromStyleCop>
</Compile>
<Compile Include="..\Shared\UnitTests\EngineTestEnvironment.cs">
<Link>EngineTestEnvironment.cs</Link>
</Compile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using Microsoft.Build.Graph;
using Microsoft.Build.Internal;
using Microsoft.Build.Shared;
using Microsoft.NET.StringTools;

namespace Microsoft.Build.Experimental.ProjectCache
{
Expand Down Expand Up @@ -549,6 +550,8 @@ static IReadOnlyCollection<ProjectGraphEntryPoint> GenerateGraphEntryPointsFromS
return Array.Empty<ProjectGraphEntryPoint>();
}

StringPool pool = new();

var graphEntryPoints = new List<ProjectGraphEntryPoint>(projectConfigurations.Count);

foreach (XmlElement projectConfiguration in projectConfigurations)
Expand All @@ -569,7 +572,7 @@ static IReadOnlyCollection<ProjectGraphEntryPoint> GenerateGraphEntryPointsFromS

string projectPath = projectPathAttribute!.Value;

(string configuration, string platform) = SolutionFile.ParseConfigurationName(projectConfiguration.InnerText, definingProjectPath, 0, solutionConfigurationXml);
(string configuration, string platform) = SolutionFile.ParseConfigurationName(projectConfiguration.InnerText.AsSpan(), definingProjectPath, 0, solutionConfigurationXml.AsSpan(), pool);

// Take the defining project global properties and override the configuration and platform.
// It's sufficient to only set Configuration and Platform.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,23 @@

using System;

#nullable disable

namespace Microsoft.Build.Construction
{
/// <summary>
/// This class represents an entry for a project configuration in a solution configuration.
/// </summary>
public sealed class ProjectConfigurationInSolution
{
private string? _fullName;

/// <summary>
/// Constructor
/// Initializes a new instance of the <see cref="ProjectConfigurationInSolution"/> class.
/// </summary>
internal ProjectConfigurationInSolution(string configurationName, string platformName, bool includeInBuild)
{
ConfigurationName = configurationName;
PlatformName = RemoveSpaceFromAnyCpuPlatform(platformName);
IncludeInBuild = includeInBuild;
FullName = SolutionConfigurationInSolution.ComputeFullName(ConfigurationName, PlatformName);
}

/// <summary>
Expand All @@ -36,7 +35,7 @@ internal ProjectConfigurationInSolution(string configurationName, string platfor
/// <summary>
/// The full name of this configuration - e.g. "Debug|Any CPU"
/// </summary>
public string FullName { get; }
public string FullName => _fullName ??= SolutionConfigurationInSolution.ComputeFullName(ConfigurationName, PlatformName);

/// <summary>
/// True if this project configuration should be built as part of its parent solution configuration
Expand Down
38 changes: 21 additions & 17 deletions src/Build/Construction/Solution/ProjectInSolution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,18 @@ public sealed class ProjectInSolution
/// </summary>
internal static readonly string[] projectNamesToDisambiguate = { "Build", "Rebuild", "Clean", "Publish" };

/// <summary>
/// Character that will be used to replace 'unclean' ones.
/// </summary>
private const char cleanCharacter = '_';

#endregion

#region Member data

private string _relativePath; // Relative from .SLN file. For example, "WindowsApplication1\WindowsApplication1.csproj"
private string _absolutePath; // Absolute path to the project file
private readonly List<string> _dependencies; // A list of strings representing the Guids of the dependent projects.
private IReadOnlyList<string> _dependenciesAsReadonly;
private Hashtable _aspNetConfigurations; // Lazily allocated collection, as this is rarely populated or read
private string _uniqueProjectName; // For example, "MySlnFolder\MySubSlnFolder\Windows_Application1"
private string _originalProjectName; // For example, "MySlnFolder\MySubSlnFolder\Windows.Application1"
private List<string> _projectReferences;

/// <summary>
/// The project configuration in given solution configuration
Expand Down Expand Up @@ -130,9 +129,6 @@ internal ProjectInSolution(SolutionFile solution)
// default to .NET Framework 3.5 if this is an old solution that doesn't explicitly say.
TargetFrameworkMoniker = ".NETFramework,Version=v3.5";

// This hashtable stores a AspNetCompilerParameters struct for each configuration name supported.
AspNetConfigurations = new Hashtable(StringComparer.OrdinalIgnoreCase);

_projectConfigurations = new Dictionary<string, ProjectConfigurationInSolution>(StringComparer.OrdinalIgnoreCase);
}

Expand Down Expand Up @@ -243,12 +239,17 @@ public IReadOnlyDictionary<string, ProjectConfigurationInSolution> ProjectConfig
/// either specified as Dependencies above, or as ProjectReferences in the
/// project file, which the solution doesn't have insight into.
/// </summary>
internal List<string> ProjectReferences { get; } = new List<string>();
internal List<string> ProjectReferences => _projectReferences ??= new();

internal SolutionFile ParentSolution { get; set; }

// Key is configuration name, value is [struct] AspNetCompilerParameters
internal Hashtable AspNetConfigurations { get; set; }
// This hashtable stores a AspNetCompilerParameters struct for each configuration name supported.
internal Hashtable AspNetConfigurations
{
get => _aspNetConfigurations ??= new Hashtable(StringComparer.OrdinalIgnoreCase);
set => _aspNetConfigurations = value;
}

internal string TargetFrameworkMoniker { get; set; }

Expand Down Expand Up @@ -391,7 +392,7 @@ internal string GetUniqueProjectName()
if (_uniqueProjectName == null)
{
// EtpSubProject and Venus projects have names that are already unique. No need to prepend the SLN folder.
if ((ProjectType == SolutionProjectType.WebProject) || (ProjectType == SolutionProjectType.EtpSubProject))
if (ProjectType is SolutionProjectType.WebProject or SolutionProjectType.EtpSubProject)
{
_uniqueProjectName = CleanseProjectName(ProjectName);
}
Expand Down Expand Up @@ -430,7 +431,7 @@ internal string GetOriginalProjectName()
if (_originalProjectName == null)
{
// EtpSubProject and Venus projects have names that are already unique. No need to prepend the SLN folder.
if ((ProjectType == SolutionProjectType.WebProject) || (ProjectType == SolutionProjectType.EtpSubProject))
if (ProjectType is SolutionProjectType.WebProject or SolutionProjectType.EtpSubProject)
{
_originalProjectName = ProjectName;
}
Expand Down Expand Up @@ -486,25 +487,28 @@ internal void UpdateUniqueProjectName(string newUniqueName)
/// </summary>
/// <param name="projectName">The name to be cleansed</param>
/// <returns>string</returns>
private static string CleanseProjectName(string projectName)
internal static string CleanseProjectName(string projectName)
{
ErrorUtilities.VerifyThrow(projectName != null, "Null strings not allowed.");

// If there are no special chars, just return the original string immediately.
// Don't even instantiate the StringBuilder.
int indexOfChar = projectName.IndexOfAny(s_charsToCleanse);

if (indexOfChar == -1)
{
// No illegal character exists in the name, so return the input unchanged.
return projectName;
}

// This is where we're going to work on the final string to return to the caller.
var cleanProjectName = new StringBuilder(projectName);
StringBuilder cleanProjectName = new(projectName);

// Replace each unclean character with a clean one
foreach (char uncleanChar in s_charsToCleanse)
while (indexOfChar != -1)
{
cleanProjectName.Replace(uncleanChar, cleanCharacter);
cleanProjectName[indexOfChar] = '_';

indexOfChar = projectName.IndexOfAny(s_charsToCleanse, indexOfChar + 1);
}

return cleanProjectName.ToString();
Expand Down
29 changes: 15 additions & 14 deletions src/Build/Construction/Solution/SolutionConfigurationInSolution.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable
using System.Collections.Immutable;

namespace Microsoft.Build.Construction
{
Expand All @@ -10,13 +10,9 @@ namespace Microsoft.Build.Construction
/// </summary>
public sealed class SolutionConfigurationInSolution
{
/// <summary>
/// Default separator between configuration and platform in configuration
/// full names
/// </summary>
internal const char ConfigurationPlatformSeparator = '|';
private static ImmutableDictionary<Key, string> _fullNameByKey = ImmutableDictionary<Key, string>.Empty;

internal static readonly char[] ConfigurationPlatformSeparatorArray = { '|' };
private string? _fullName;

/// <summary>
/// Constructor
Expand All @@ -25,7 +21,6 @@ internal SolutionConfigurationInSolution(string configurationName, string platfo
{
ConfigurationName = configurationName;
PlatformName = platformName;
FullName = ComputeFullName(configurationName, platformName);
}

/// <summary>
Expand All @@ -41,20 +36,26 @@ internal SolutionConfigurationInSolution(string configurationName, string platfo
/// <summary>
/// The full name of this configuration - e.g. "Debug|Any CPU"
/// </summary>
public string FullName { get; }
public string FullName => _fullName ??= ComputeFullName(ConfigurationName, PlatformName);

/// <summary>
/// Given a configuration name and a platform name, compute the full name
/// of this configuration
/// Given a configuration name and a platform name, compute the full name
/// of this configuration.
/// </summary>
internal static string ComputeFullName(string configurationName, string platformName)
{
// Some configurations don't have the platform part
if (!string.IsNullOrEmpty(platformName))
if (string.IsNullOrEmpty(platformName))
{
return $"{configurationName}{ConfigurationPlatformSeparator}{platformName}";
return configurationName;
}
return configurationName;

return ImmutableInterlocked.GetOrAdd(
ref _fullNameByKey,
new Key(configurationName, platformName),
static key => $"{key.Configuration}|{key.Platform}");
}

private record struct Key(string Configuration, string Platform);
}
}
Loading