Skip to content

Commit

Permalink
Cleanup: Delete NGen of T (#9263)
Browse files Browse the repository at this point in the history
Context
The sole purpose of introducing the type seems to have been silencing a legacy code analyzer rule. The rule does not exist anymore / has not been brought over to Roslyn (dotnet/roslyn-analyzers#722) and it's now hurting performance, if anything. Types like HashSet<int> are part of the mscorlib native image and it's wasteful to duplicate the code in our binaries. The rest is handled by IBC/OptProf.

Changes Made
Deleted NGen and its uses.

Testing
Experimental insertion to confirm no regressions.
  • Loading branch information
ladipro committed Oct 17, 2023
1 parent e493e7a commit 2016d60
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 71 deletions.
14 changes: 7 additions & 7 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable
/// <summary>
/// Set of active nodes in the system.
/// </summary>
private readonly HashSet<NGen<int>> _activeNodes;
private readonly HashSet<int> _activeNodes;

/// <summary>
/// Event signalled when all nodes have shutdown.
Expand All @@ -128,7 +128,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable
/// <summary>
/// Mapping of nodes to the configurations they know about.
/// </summary>
private readonly Dictionary<NGen<int>, HashSet<NGen<int>>> _nodeIdToKnownConfigurations;
private readonly Dictionary<int, HashSet<int>> _nodeIdToKnownConfigurations;

/// <summary>
/// Flag indicating if we are currently shutting down. When set, we stop processing packets other than NodeShutdown.
Expand Down Expand Up @@ -301,9 +301,9 @@ public BuildManager(string hostName)
_buildSubmissions = new Dictionary<int, BuildSubmission>();
_graphBuildSubmissions = new Dictionary<int, GraphBuildSubmission>();
_noActiveSubmissionsEvent = new AutoResetEvent(true);
_activeNodes = new HashSet<NGen<int>>();
_activeNodes = new HashSet<int>();
_noNodesActiveEvent = new AutoResetEvent(true);
_nodeIdToKnownConfigurations = new Dictionary<NGen<int>, HashSet<NGen<int>>>();
_nodeIdToKnownConfigurations = new Dictionary<int, HashSet<int>>();
_unnamedProjectInstanceToNames = new Dictionary<ProjectInstance, string>();
_nextUnnamedProjectId = 1;
_componentFactories = new BuildComponentFactoryCollection(this);
Expand Down Expand Up @@ -2394,9 +2394,9 @@ private void HandleConfigurationRequest(int node, BuildRequestConfiguration unre

var response = new BuildRequestConfigurationResponse(unresolvedConfiguration.ConfigurationId, resolvedConfiguration.ConfigurationId, resolvedConfiguration.ResultsNodeId);

if (!_nodeIdToKnownConfigurations.TryGetValue(node, out HashSet<NGen<int>> configurationsOnNode))
if (!_nodeIdToKnownConfigurations.TryGetValue(node, out HashSet<int> configurationsOnNode))
{
configurationsOnNode = new HashSet<NGen<int>>();
configurationsOnNode = new HashSet<int>();
_nodeIdToKnownConfigurations[node] = configurationsOnNode;
}

Expand Down Expand Up @@ -2664,7 +2664,7 @@ private void PerformSchedulingActions(IEnumerable<ScheduleResponse> responses)
// of which nodes have had configurations specifically assigned to them for building. However, a node may
// have created a configuration based on a build request it needs to wait on. In this
// case we need not send the configuration since it will already have been mapped earlier.
if (!_nodeIdToKnownConfigurations.TryGetValue(response.NodeId, out HashSet<NGen<int>> configurationsOnNode) ||
if (!_nodeIdToKnownConfigurations.TryGetValue(response.NodeId, out HashSet<int> configurationsOnNode) ||
!configurationsOnNode.Contains(response.BuildRequest.ConfigurationId))
{
IConfigCache configCache = _componentFactories.GetComponent(BuildComponentType.ConfigCache) as IConfigCache;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,7 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu
lock (issuingEntry.GlobalLock)
{
var existingResultsToReport = new List<BuildResult>();
var unresolvedConfigurationsAdded = new HashSet<NGen<int>>();
var unresolvedConfigurationsAdded = new HashSet<int>();

foreach (FullyQualifiedBuildRequest request in newRequests)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Build/Evaluation/Evaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ internal class Evaluator<P, I, M, D>
/// Dictionary of project full paths and a boolean that indicates whether at least one
/// of their targets has the "Returns" attribute set.
/// </summary>
private readonly Dictionary<ProjectRootElement, NGen<bool>> _projectSupportsReturnsAttribute;
private readonly Dictionary<ProjectRootElement, bool> _projectSupportsReturnsAttribute;

/// <summary>
/// The Project Xml to be evaluated.
Expand Down Expand Up @@ -253,7 +253,7 @@ private Evaluator(
_targetElements = new List<ProjectTargetElement>();
_importsSeen = new Dictionary<string, ProjectImportElement>(StringComparer.OrdinalIgnoreCase);
_initialTargetsList = new List<string>();
_projectSupportsReturnsAttribute = new Dictionary<ProjectRootElement, NGen<bool>>();
_projectSupportsReturnsAttribute = new Dictionary<ProjectRootElement, bool>();
_projectRootElement = projectRootElement;
_loadSettings = loadSettings;
_maxNodeCount = maxNodeCount;
Expand Down Expand Up @@ -901,7 +901,7 @@ private void PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
break;
case ProjectTargetElement target:
// Defaults to false
_projectSupportsReturnsAttribute.TryGetValue(currentProjectOrImport, out NGen<bool> projectSupportsReturnsAttribute);
_projectSupportsReturnsAttribute.TryGetValue(currentProjectOrImport, out bool projectSupportsReturnsAttribute);

_projectSupportsReturnsAttribute[currentProjectOrImport] = projectSupportsReturnsAttribute || (target.Returns != null);
_targetElements.Add(target);
Expand Down
3 changes: 0 additions & 3 deletions src/Build/Microsoft.Build.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@
<Compile Include="..\Shared\IConstrainedEqualityComparer.cs">
<Link>IConstrainedEqualityComparer.cs</Link>
</Compile>
<Compile Include="..\Shared\NGen.cs">
<Link>SharedUtilities\NGen.cs</Link>
</Compile>
<Compile Include="..\Shared\PropertyParser.cs">
<Link>BackEnd\Components\RequestBuilder\IntrinsicTasks\PropertyParser.cs</Link>
</Compile>
Expand Down
50 changes: 0 additions & 50 deletions src/Shared/NGen.cs

This file was deleted.

2 changes: 1 addition & 1 deletion src/Tasks/ManifestUtil/ApplicationManifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ private void ValidateReferencesForClickOnceApplication()
{
int t1 = Environment.TickCount;
bool isPartialTrust = !TrustInfo.IsFullTrust;
var targetPathList = new Dictionary<string, NGen<bool>>();
var targetPathList = new Dictionary<string, bool>();

foreach (AssemblyReference assembly in AssemblyReferences)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/ManifestUtil/Manifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ private void ValidateReferences()
return;
}

var identityList = new Dictionary<string, NGen<bool>>();
var identityList = new Dictionary<string, bool>();
foreach (AssemblyReference assembly in AssemblyReferences)
{
if (assembly.AssemblyIdentity != null)
Expand Down
3 changes: 0 additions & 3 deletions src/Tasks/Microsoft.Build.Tasks.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@
<Compile Include="..\Shared\FileDelegates.cs">
<Link>FileDelegates.cs</Link>
</Compile>
<Compile Include="..\Shared\NGen.cs">
<Link>NGen.cs</Link>
</Compile>
<Compile Include="..\Shared\IConstrainedEqualityComparer.cs" />
<Compile Include="..\Shared\PropertyParser.cs">
<Link>PropertyParser.cs</Link>
Expand Down
4 changes: 2 additions & 2 deletions src/Tasks/RedistList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal sealed class RedistList
/// When we check to see if an assembly is in this redist list we want to cache it so that if we ask again we do not
/// have to re-scan bits of the redist list and do the assemblynameExtension comparisons.
/// </summary>
private readonly ConcurrentDictionary<AssemblyNameExtension, NGen<bool>> _assemblyNameInRedist = new ConcurrentDictionary<AssemblyNameExtension, NGen<bool>>(AssemblyNameComparer.GenericComparer);
private readonly ConcurrentDictionary<AssemblyNameExtension, bool> _assemblyNameInRedist = new ConcurrentDictionary<AssemblyNameExtension, bool>(AssemblyNameComparer.GenericComparer);

/// <summary>
/// AssemblyName to unified assemblyName. We make this kind of call a lot and also will ask for the same name multiple times.
Expand Down Expand Up @@ -431,7 +431,7 @@ public bool FrameworkAssemblyEntryInRedist(AssemblyNameExtension assemblyName)
{
ErrorUtilities.VerifyThrowArgumentNull(assemblyName, nameof(assemblyName));

if (!_assemblyNameInRedist.TryGetValue(assemblyName, out NGen<bool> isAssemblyNameInRedist))
if (!_assemblyNameInRedist.TryGetValue(assemblyName, out bool isAssemblyNameInRedist))
{
string simpleName = GetSimpleName(assemblyName.Name);
if (_simpleNameMap.TryGetValue(simpleName, out int index))
Expand Down

0 comments on commit 2016d60

Please sign in to comment.