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

Remove ContainsKey where relevant #6282

Merged
merged 2 commits into from Mar 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -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