Skip to content

Commit

Permalink
Reduce dictionary accesses
Browse files Browse the repository at this point in the history
p2

p3

p4

p5

P6

p7
  • Loading branch information
Forgind committed Mar 23, 2021
1 parent 97463e3 commit 928968c
Show file tree
Hide file tree
Showing 45 changed files with 164 additions and 312 deletions.
7 changes: 2 additions & 5 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Expand Up @@ -2423,10 +2423,8 @@ private void ReportResultsToSubmission(BuildResult result)
lock (_syncLock)
{
// The build submission has not already been completed.
if (_buildSubmissions.ContainsKey(result.SubmissionId))
if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmission submission))
{
BuildSubmission submission = _buildSubmissions[result.SubmissionId];

/* If the request failed because we caught an exception from the loggers, we can assume we will receive no more logging messages for
* this submission, therefore set the logging as complete. InternalLoggerExceptions are unhandled exceptions from the logger. If the logger author does
* not handle an exception the eventsource wraps all exceptions (except a logging exception) into an internal logging exception.
Expand Down Expand Up @@ -2459,9 +2457,8 @@ private void ReportResultsToSubmission(GraphBuildResult result)
lock (_syncLock)
{
// The build submission has not already been completed.
if (_graphBuildSubmissions.ContainsKey(result.SubmissionId))
if (_graphBuildSubmissions.TryGetValue(result.SubmissionId, out GraphBuildSubmission submission))
{
GraphBuildSubmission submission = _graphBuildSubmissions[result.SubmissionId];
submission.CompleteResults(result);

_overallBuildSuccess &= submission.BuildResult.OverallResult == BuildResultCode.Success;
Expand Down
5 changes: 1 addition & 4 deletions src/Build/BackEnd/BuildManager/LegacyThreadingData.cs
Expand Up @@ -104,10 +104,7 @@ internal void UnregisterSubmissionForLegacyThread(int submissionId)
{
ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should have been previously registered with LegacyThreadingData", submissionId);

if (_legacyThreadingEventsById.ContainsKey(submissionId))
{
_legacyThreadingEventsById.Remove(submissionId);
}
_legacyThreadingEventsById.Remove(submissionId);
}
}

Expand Down
Expand Up @@ -230,12 +230,12 @@ public bool ResolveConfigurationRequest(int unresolvedConfigId, int configId)
{
lock (GlobalLock)
{
if (_unresolvedConfigurations?.ContainsKey(unresolvedConfigId) != true)
List<BuildRequest> requests = null;
if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out requests) != true)
{
return false;
}

List<BuildRequest> requests = _unresolvedConfigurations[unresolvedConfigId];
_unresolvedConfigurations.Remove(unresolvedConfigId);

if (_unresolvedConfigurations.Count == 0)
Expand Down
13 changes: 5 additions & 8 deletions src/Build/BackEnd/Components/Caching/ResultsCache.cs
Expand Up @@ -55,15 +55,15 @@ public void AddResult(BuildResult result)
{
lock (_resultsByConfiguration)
{
if (_resultsByConfiguration.ContainsKey(result.ConfigurationId))
if (_resultsByConfiguration.TryGetValue(result.ConfigurationId, out BuildResult buildResult))
{
if (Object.ReferenceEquals(_resultsByConfiguration[result.ConfigurationId], result))
if (Object.ReferenceEquals(buildResult, result))
{
// Merging results would be meaningless as we would be merging the object with itself.
return;
}

_resultsByConfiguration[result.ConfigurationId].MergeResults(result);
buildResult.MergeResults(result);
}
else
{
Expand Down Expand Up @@ -105,9 +105,8 @@ public BuildResult GetResultForRequest(BuildRequest request)

lock (_resultsByConfiguration)
{
if (_resultsByConfiguration.ContainsKey(request.ConfigurationId))
if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult result))
{
BuildResult result = _resultsByConfiguration[request.ConfigurationId];
foreach (string target in request.Targets)
{
ErrorUtilities.VerifyThrow(result.HasResultsForTarget(target), "No results in cache for target " + target);
Expand Down Expand Up @@ -159,10 +158,8 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> co

lock (_resultsByConfiguration)
{
if (_resultsByConfiguration.ContainsKey(request.ConfigurationId))
if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults))
{
BuildResult allResults = _resultsByConfiguration[request.ConfigurationId];

// Check for targets explicitly specified.
bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss);

Expand Down
Expand Up @@ -265,9 +265,9 @@ public void UnregisterPacketHandler(NodePacketType packetType)
/// <param name="translator">The translator containing the data from which the packet should be reconstructed.</param>
public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITranslator translator)
{
if (_nodeIdToPacketFactory.ContainsKey(nodeId))
if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory))
{
_nodeIdToPacketFactory[nodeId].DeserializeAndRoutePacket(nodeId, packetType, translator);
nodePacketFactory.DeserializeAndRoutePacket(nodeId, packetType, translator);
}
else
{
Expand All @@ -282,9 +282,9 @@ public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITr
/// <param name="packet">The packet to route.</param>
public void RoutePacket(int nodeId, INodePacket packet)
{
if (_nodeIdToPacketFactory.ContainsKey(nodeId))
if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory))
{
_nodeIdToPacketFactory[nodeId].RoutePacket(nodeId, packet);
nodePacketFactory.RoutePacket(nodeId, packet);
}
else
{
Expand All @@ -304,9 +304,9 @@ public void RoutePacket(int nodeId, INodePacket packet)
/// <param name="packet">The packet.</param>
public void PacketReceived(int node, INodePacket packet)
{
if (_nodeIdToPacketHandler.ContainsKey(node))
if (_nodeIdToPacketHandler.TryGetValue(node, out INodePacketHandler packetHandler))
{
_nodeIdToPacketHandler[node].PacketReceived(node, packet);
packetHandler.PacketReceived(node, packet);
}
else
{
Expand Down Expand Up @@ -484,10 +484,7 @@ internal bool AcquireAndSetUpHost(HandshakeOptions hostContext, INodePacketFacto
/// </summary>
internal void DisconnectFromHost(HandshakeOptions hostContext)
{
ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.ContainsKey((int)hostContext) && _nodeIdToPacketHandler.ContainsKey((int)hostContext), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?");

_nodeIdToPacketFactory.Remove((int)hostContext);
_nodeIdToPacketHandler.Remove((int)hostContext);
ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.Remove((int)hostContext) && _nodeIdToPacketHandler.Remove((int)hostContext), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?");
}

/// <summary>
Expand Down
Expand Up @@ -626,12 +626,10 @@ public void LogProjectFinished(BuildEventContext projectBuildEventContext, strin
ProcessLoggingEvent(buildEvent);

// PERF: Not using VerifyThrow to avoid boxing of projectBuildEventContext.ProjectContextId in the non-error case.
if (!_projectFileMap.ContainsKey(projectBuildEventContext.ProjectContextId))
if (!_projectFileMap.Remove(projectBuildEventContext.ProjectContextId))
{
ErrorUtilities.ThrowInternalError("ContextID {0} for project {1} should be in the ID-to-file mapping!", projectBuildEventContext.ProjectContextId, projectFile);
}

_projectFileMap.Remove(projectBuildEventContext.ProjectContextId);
}
}

Expand Down
Expand Up @@ -661,10 +661,8 @@ bool runEachTargetSeparately

foreach (string targetName in nonNullTargetList)
{
if (targetOutputsPerProject[i].ContainsKey(targetName))
if (targetOutputsPerProject[i].TryGetValue(targetName, out ITaskItem[] outputItemsFromTarget))
{
ITaskItem[] outputItemsFromTarget = targetOutputsPerProject[i][targetName];

foreach (ITaskItem outputItemFromTarget in outputItemsFromTarget)
{
// No need to rebase if the calling project is the same as the callee project
Expand Down
10 changes: 5 additions & 5 deletions src/Build/BackEnd/Components/RequestBuilder/Lookup.cs
Expand Up @@ -210,13 +210,13 @@ internal List<string> GetPropertyOverrideMessages(Dictionary<string, string> loo

string propertyName = property.Name;
// If the hash contains the property name, output a messages that displays the previous property value and the new property value
if (lookupHash.ContainsKey(propertyName))
if (lookupHash.TryGetValue(propertyName, out string propertyValue))
{
if (errorMessages == null)
{
errorMessages = new List<string>();
}
errorMessages.Add(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyOutputOverridden", propertyName, EscapingUtilities.UnescapeAll(lookupHash[propertyName]), property.EvaluatedValue));
errorMessages.Add(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyOutputOverridden", propertyName, EscapingUtilities.UnescapeAll(propertyValue), property.EvaluatedValue));
}

// Set the value of the hash to the new property value
Expand Down Expand Up @@ -945,10 +945,10 @@ private void MustNotBeInTable(ItemDictionary<ProjectItemInstance> table, Project
/// </summary>
private void MustNotBeInTable(ItemTypeToItemsMetadataUpdateDictionary table, ProjectItemInstance item)
{
if (table?.ContainsKey(item.ItemType) == true)
ItemsMetadataUpdateDictionary tableOfItemsOfSameType = null;
if (table?.TryGetValue(item.ItemType, out tableOfItemsOfSameType) == true)
{
ItemsMetadataUpdateDictionary tableOfItemsOfSameType = table[item.ItemType];
if (tableOfItemsOfSameType != null)
if (tableOfItemsOfSameType is not null)
{
ErrorUtilities.VerifyThrow(!tableOfItemsOfSameType.ContainsKey(item), "Item should not be in table");
}
Expand Down
22 changes: 8 additions & 14 deletions src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs
Expand Up @@ -142,16 +142,16 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext

foreach (string targetName in targetNames)
{
var targetExists = _projectInstance.Targets.ContainsKey(targetName);
var targetExists = _projectInstance.Targets.TryGetValue(targetName, out ProjectTargetInstance targetInstance);
if (!targetExists && entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets))
{
_projectLoggingContext.LogComment(Framework.MessageImportance.Low,
"TargetSkippedWhenSkipNonexistentTargets", targetName);

continue;
}

targets.Add(new TargetSpecification(targetName, targetExists ? _projectInstance.Targets[targetName].Location : _projectInstance.ProjectFileLocation));
else
{
targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
}
}

// Push targets onto the stack. This method will reverse their push order so that they
Expand Down Expand Up @@ -484,13 +484,7 @@ private async Task ProcessTargetStack(ITaskBuilder taskBuilder)
}
catch
{
if (_requestEntry.RequestConfiguration.ActivelyBuildingTargets.ContainsKey(
currentTargetEntry.Name))
{
_requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry
.Name);
}

_requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry.Name);
throw;
}
}
Expand Down Expand Up @@ -764,7 +758,7 @@ private void ComputeAfterTargetFailures(string[] targetNames)
{
foreach (string targetName in targetNames)
{
if (_buildResult.ResultsByTarget.ContainsKey(targetName))
if (_buildResult.ResultsByTarget.TryGetValue(targetName, out TargetResult targetBuildResult))
{
// Queue of targets waiting to be processed, seeded with the specific target for which we're computing AfterTargetsHaveFailed.
var targetsToCheckForAfterTargets = new Queue<string>();
Expand All @@ -785,7 +779,7 @@ private void ComputeAfterTargetFailures(string[] targetNames)
if (result?.ResultCode == TargetResultCode.Failure && !result.TargetFailureDoesntCauseBuildFailure)
{
// Mark the target as having an after target failed, and break the loop to move to the next target.
_buildResult.ResultsByTarget[targetName].AfterTargetsHaveFailed = true;
targetBuildResult.AfterTargetsHaveFailed = true;
targetsToCheckForAfterTargets = null;
break;
}
Expand Down
4 changes: 1 addition & 3 deletions src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs
Expand Up @@ -629,9 +629,7 @@ private void CleanupForCircularDependencyAndThrow(SchedulableRequest requestCaus
/// </summary>
internal void DisconnectRequestWeAreBlockedBy(BlockingRequestKey blockingRequestKey)
{
ErrorUtilities.VerifyThrow(_requestsWeAreBlockedBy.ContainsKey(blockingRequestKey), "We are not blocked by the specified request.");

SchedulableRequest unblockingRequest = _requestsWeAreBlockedBy[blockingRequestKey];
ErrorUtilities.VerifyThrow(_requestsWeAreBlockedBy.TryGetValue(blockingRequestKey, out SchedulableRequest unblockingRequest), "We are not blocked by the specified request.");
ErrorUtilities.VerifyThrow(unblockingRequest._requestsWeAreBlocking.Contains(this), "The request unblocking us doesn't think it is blocking us.");

_requestsWeAreBlockedBy.Remove(blockingRequestKey);
Expand Down
4 changes: 2 additions & 2 deletions src/Build/BackEnd/Components/Scheduler/SchedulingData.cs
Expand Up @@ -360,9 +360,9 @@ public void UpdateFromState(SchedulableRequest request, SchedulableRequestState
ErrorUtilities.VerifyThrow(_configurationToRequests.ContainsKey(request.BuildRequest.ConfigurationId), "Configuration {0} never had requests assigned to it.", request.BuildRequest.ConfigurationId);
ErrorUtilities.VerifyThrow(_configurationToRequests[request.BuildRequest.ConfigurationId].Count > 0, "Configuration {0} has no requests assigned to it.", request.BuildRequest.ConfigurationId);
_configurationToRequests[request.BuildRequest.ConfigurationId].Remove(request);
if (_scheduledRequestsByNode.ContainsKey(request.AssignedNode))
if (_scheduledRequestsByNode.TryGetValue(request.AssignedNode, out var requests))
{
_scheduledRequestsByNode[request.AssignedNode].Remove(request);
requests.Remove(request);
}

request.EndTime = EventTime;
Expand Down
4 changes: 2 additions & 2 deletions src/Build/BackEnd/Shared/BuildResult.cs
Expand Up @@ -470,9 +470,9 @@ public void AddResultsForTarget(string target, TargetResult result)
_resultsByTarget ??= CreateTargetResultDictionary(1);
}

if (_resultsByTarget.ContainsKey(target))
if (_resultsByTarget.TryGetValue(target, out TargetResult targetResult))
{
ErrorUtilities.VerifyThrow(_resultsByTarget[target].ResultCode == TargetResultCode.Skipped, "Items already exist for target {0}.", target);
ErrorUtilities.VerifyThrow(targetResult.ResultCode == TargetResultCode.Skipped, "Items already exist for target {0}.", target);
}

_resultsByTarget[target] = result;
Expand Down
12 changes: 5 additions & 7 deletions src/Build/Construction/Solution/SolutionFile.cs
Expand Up @@ -617,7 +617,7 @@ internal void ParseSolution()
}

// Detect collision caused by unique name's normalization
if (projectsByUniqueName.ContainsKey(uniqueName))
if (projectsByUniqueName.TryGetValue(uniqueName, out ProjectInSolution project))
{
// Did normalization occur in the current project?
if (uniqueName != proj.ProjectName)
Expand All @@ -628,16 +628,14 @@ internal void ParseSolution()
uniqueName = tempUniqueName;
}
// Did normalization occur in a previous project?
else if (uniqueName != projectsByUniqueName[uniqueName].ProjectName)
else if (uniqueName != project.ProjectName)
{
var projTemp = projectsByUniqueName[uniqueName];

// Generates a new unique name
string tempUniqueName = $"{uniqueName}_{projTemp.GetProjectGuidWithoutCurlyBrackets()}";
projTemp.UpdateUniqueProjectName(tempUniqueName);
string tempUniqueName = $"{uniqueName}_{project.GetProjectGuidWithoutCurlyBrackets()}";
project.UpdateUniqueProjectName(tempUniqueName);

projectsByUniqueName.Remove(uniqueName);
projectsByUniqueName.Add(tempUniqueName, projTemp);
projectsByUniqueName.Add(tempUniqueName, project);
}
}

Expand Down

0 comments on commit 928968c

Please sign in to comment.