Skip to content

Commit c2e41ab

Browse files
ForgindForgind
authored
Remove ContainsKey where relevant (#6282)
Reduce dictionary accesses by merging ContainsKey calls with subsequent gets or removes.
1 parent 9052065 commit c2e41ab

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+163
-308
lines changed

‎src/Build/BackEnd/BuildManager/BuildManager.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,10 +2453,8 @@ private void ReportResultsToSubmission(BuildResult result)
24532453
lock (_syncLock)
24542454
{
24552455
// The build submission has not already been completed.
2456-
if (_buildSubmissions.ContainsKey(result.SubmissionId))
2456+
if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmission submission))
24572457
{
2458-
BuildSubmission submission = _buildSubmissions[result.SubmissionId];
2459-
24602458
/* If the request failed because we caught an exception from the loggers, we can assume we will receive no more logging messages for
24612459
* this submission, therefore set the logging as complete. InternalLoggerExceptions are unhandled exceptions from the logger. If the logger author does
24622460
* not handle an exception the eventsource wraps all exceptions (except a logging exception) into an internal logging exception.
@@ -2489,9 +2487,8 @@ private void ReportResultsToSubmission(GraphBuildResult result)
24892487
lock (_syncLock)
24902488
{
24912489
// The build submission has not already been completed.
2492-
if (_graphBuildSubmissions.ContainsKey(result.SubmissionId))
2490+
if (_graphBuildSubmissions.TryGetValue(result.SubmissionId, out GraphBuildSubmission submission))
24932491
{
2494-
GraphBuildSubmission submission = _graphBuildSubmissions[result.SubmissionId];
24952492
submission.CompleteResults(result);
24962493

24972494
_overallBuildSuccess &= submission.BuildResult.OverallResult == BuildResultCode.Success;

‎src/Build/BackEnd/BuildManager/LegacyThreadingData.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,7 @@ internal void UnregisterSubmissionForLegacyThread(int submissionId)
104104
{
105105
ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should have been previously registered with LegacyThreadingData", submissionId);
106106

107-
if (_legacyThreadingEventsById.ContainsKey(submissionId))
108-
{
109-
_legacyThreadingEventsById.Remove(submissionId);
110-
}
107+
_legacyThreadingEventsById.Remove(submissionId);
111108
}
112109
}
113110

‎src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,12 @@ public bool ResolveConfigurationRequest(int unresolvedConfigId, int configId)
230230
{
231231
lock (GlobalLock)
232232
{
233-
if (_unresolvedConfigurations?.ContainsKey(unresolvedConfigId) != true)
233+
List<BuildRequest> requests = null;
234+
if (_unresolvedConfigurations?.TryGetValue(unresolvedConfigId, out requests) != true)
234235
{
235236
return false;
236237
}
237238

238-
List<BuildRequest> requests = _unresolvedConfigurations[unresolvedConfigId];
239239
_unresolvedConfigurations.Remove(unresolvedConfigId);
240240

241241
if (_unresolvedConfigurations.Count == 0)

‎src/Build/BackEnd/Components/Caching/ResultsCache.cs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ public void AddResult(BuildResult result)
5555
{
5656
lock (_resultsByConfiguration)
5757
{
58-
if (_resultsByConfiguration.ContainsKey(result.ConfigurationId))
58+
if (_resultsByConfiguration.TryGetValue(result.ConfigurationId, out BuildResult buildResult))
5959
{
60-
if (Object.ReferenceEquals(_resultsByConfiguration[result.ConfigurationId], result))
60+
if (Object.ReferenceEquals(buildResult, result))
6161
{
6262
// Merging results would be meaningless as we would be merging the object with itself.
6363
return;
6464
}
6565

66-
_resultsByConfiguration[result.ConfigurationId].MergeResults(result);
66+
buildResult.MergeResults(result);
6767
}
6868
else
6969
{
@@ -105,9 +105,8 @@ public BuildResult GetResultForRequest(BuildRequest request)
105105

106106
lock (_resultsByConfiguration)
107107
{
108-
if (_resultsByConfiguration.ContainsKey(request.ConfigurationId))
108+
if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult result))
109109
{
110-
BuildResult result = _resultsByConfiguration[request.ConfigurationId];
111110
foreach (string target in request.Targets)
112111
{
113112
ErrorUtilities.VerifyThrow(result.HasResultsForTarget(target), "No results in cache for target " + target);
@@ -159,10 +158,8 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> co
159158

160159
lock (_resultsByConfiguration)
161160
{
162-
if (_resultsByConfiguration.ContainsKey(request.ConfigurationId))
161+
if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults))
163162
{
164-
BuildResult allResults = _resultsByConfiguration[request.ConfigurationId];
165-
166163
// Check for targets explicitly specified.
167164
bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss);
168165

‎src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,9 @@ public void UnregisterPacketHandler(NodePacketType packetType)
265265
/// <param name="translator">The translator containing the data from which the packet should be reconstructed.</param>
266266
public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITranslator translator)
267267
{
268-
if (_nodeIdToPacketFactory.ContainsKey(nodeId))
268+
if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory))
269269
{
270-
_nodeIdToPacketFactory[nodeId].DeserializeAndRoutePacket(nodeId, packetType, translator);
270+
nodePacketFactory.DeserializeAndRoutePacket(nodeId, packetType, translator);
271271
}
272272
else
273273
{
@@ -282,9 +282,9 @@ public void DeserializeAndRoutePacket(int nodeId, NodePacketType packetType, ITr
282282
/// <param name="packet">The packet to route.</param>
283283
public void RoutePacket(int nodeId, INodePacket packet)
284284
{
285-
if (_nodeIdToPacketFactory.ContainsKey(nodeId))
285+
if (_nodeIdToPacketFactory.TryGetValue(nodeId, out INodePacketFactory nodePacketFactory))
286286
{
287-
_nodeIdToPacketFactory[nodeId].RoutePacket(nodeId, packet);
287+
nodePacketFactory.RoutePacket(nodeId, packet);
288288
}
289289
else
290290
{
@@ -304,9 +304,9 @@ public void RoutePacket(int nodeId, INodePacket packet)
304304
/// <param name="packet">The packet.</param>
305305
public void PacketReceived(int node, INodePacket packet)
306306
{
307-
if (_nodeIdToPacketHandler.ContainsKey(node))
307+
if (_nodeIdToPacketHandler.TryGetValue(node, out INodePacketHandler packetHandler))
308308
{
309-
_nodeIdToPacketHandler[node].PacketReceived(node, packet);
309+
packetHandler.PacketReceived(node, packet);
310310
}
311311
else
312312
{

‎src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -626,12 +626,10 @@ public void LogProjectFinished(BuildEventContext projectBuildEventContext, strin
626626
ProcessLoggingEvent(buildEvent);
627627

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

‎src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -661,10 +661,8 @@ internal static async Task<bool> ExecuteTargets(
661661

662662
foreach (string targetName in nonNullTargetList)
663663
{
664-
if (targetOutputsPerProject[i].ContainsKey(targetName))
664+
if (targetOutputsPerProject[i].TryGetValue(targetName, out ITaskItem[] outputItemsFromTarget))
665665
{
666-
ITaskItem[] outputItemsFromTarget = targetOutputsPerProject[i][targetName];
667-
668666
foreach (ITaskItem outputItemFromTarget in outputItemsFromTarget)
669667
{
670668
// No need to rebase if the calling project is the same as the callee project

‎src/Build/BackEnd/Components/RequestBuilder/Lookup.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,13 @@ internal List<string> GetPropertyOverrideMessages(Dictionary<string, string> loo
210210

211211
string propertyName = property.Name;
212212
// If the hash contains the property name, output a messages that displays the previous property value and the new property value
213-
if (lookupHash.ContainsKey(propertyName))
213+
if (lookupHash.TryGetValue(propertyName, out string propertyValue))
214214
{
215215
if (errorMessages == null)
216216
{
217217
errorMessages = new List<string>();
218218
}
219-
errorMessages.Add(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyOutputOverridden", propertyName, EscapingUtilities.UnescapeAll(lookupHash[propertyName]), property.EvaluatedValue));
219+
errorMessages.Add(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyOutputOverridden", propertyName, EscapingUtilities.UnescapeAll(propertyValue), property.EvaluatedValue));
220220
}
221221

222222
// Set the value of the hash to the new property value
@@ -945,10 +945,10 @@ private void MustNotBeInTable(ItemDictionary<ProjectItemInstance> table, Project
945945
/// </summary>
946946
private void MustNotBeInTable(ItemTypeToItemsMetadataUpdateDictionary table, ProjectItemInstance item)
947947
{
948-
if (table?.ContainsKey(item.ItemType) == true)
948+
ItemsMetadataUpdateDictionary tableOfItemsOfSameType = null;
949+
if (table?.TryGetValue(item.ItemType, out tableOfItemsOfSameType) == true)
949950
{
950-
ItemsMetadataUpdateDictionary tableOfItemsOfSameType = table[item.ItemType];
951-
if (tableOfItemsOfSameType != null)
951+
if (tableOfItemsOfSameType is not null)
952952
{
953953
ErrorUtilities.VerifyThrow(!tableOfItemsOfSameType.ContainsKey(item), "Item should not be in table");
954954
}

‎src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,16 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext
142142

143143
foreach (string targetName in targetNames)
144144
{
145-
var targetExists = _projectInstance.Targets.ContainsKey(targetName);
145+
var targetExists = _projectInstance.Targets.TryGetValue(targetName, out ProjectTargetInstance targetInstance);
146146
if (!targetExists && entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets))
147147
{
148148
_projectLoggingContext.LogComment(Framework.MessageImportance.Low,
149149
"TargetSkippedWhenSkipNonexistentTargets", targetName);
150-
151-
continue;
152150
}
153-
154-
targets.Add(new TargetSpecification(targetName, targetExists ? _projectInstance.Targets[targetName].Location : _projectInstance.ProjectFileLocation));
151+
else
152+
{
153+
targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
154+
}
155155
}
156156

157157
// Push targets onto the stack. This method will reverse their push order so that they
@@ -502,13 +502,7 @@ await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, t
502502
}
503503
catch
504504
{
505-
if (_requestEntry.RequestConfiguration.ActivelyBuildingTargets.ContainsKey(
506-
currentTargetEntry.Name))
507-
{
508-
_requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry
509-
.Name);
510-
}
511-
505+
_requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry.Name);
512506
throw;
513507
}
514508
}
@@ -782,7 +776,7 @@ private void ComputeAfterTargetFailures(string[] targetNames)
782776
{
783777
foreach (string targetName in targetNames)
784778
{
785-
if (_buildResult.ResultsByTarget.ContainsKey(targetName))
779+
if (_buildResult.ResultsByTarget.TryGetValue(targetName, out TargetResult targetBuildResult))
786780
{
787781
// Queue of targets waiting to be processed, seeded with the specific target for which we're computing AfterTargetsHaveFailed.
788782
var targetsToCheckForAfterTargets = new Queue<string>();
@@ -803,7 +797,7 @@ private void ComputeAfterTargetFailures(string[] targetNames)
803797
if (result?.ResultCode == TargetResultCode.Failure && !result.TargetFailureDoesntCauseBuildFailure)
804798
{
805799
// Mark the target as having an after target failed, and break the loop to move to the next target.
806-
_buildResult.ResultsByTarget[targetName].AfterTargetsHaveFailed = true;
800+
targetBuildResult.AfterTargetsHaveFailed = true;
807801
targetsToCheckForAfterTargets = null;
808802
break;
809803
}

‎src/Build/BackEnd/Components/Scheduler/SchedulableRequest.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -634,9 +634,7 @@ private void CleanupForCircularDependencyAndThrow(SchedulableRequest requestCaus
634634
/// </summary>
635635
internal void DisconnectRequestWeAreBlockedBy(BlockingRequestKey blockingRequestKey)
636636
{
637-
ErrorUtilities.VerifyThrow(_requestsWeAreBlockedBy.ContainsKey(blockingRequestKey), "We are not blocked by the specified request.");
638-
639-
SchedulableRequest unblockingRequest = _requestsWeAreBlockedBy[blockingRequestKey];
637+
ErrorUtilities.VerifyThrow(_requestsWeAreBlockedBy.TryGetValue(blockingRequestKey, out SchedulableRequest unblockingRequest), "We are not blocked by the specified request.");
640638
ErrorUtilities.VerifyThrow(unblockingRequest._requestsWeAreBlocking.Contains(this), "The request unblocking us doesn't think it is blocking us.");
641639

642640
_requestsWeAreBlockedBy.Remove(blockingRequestKey);

0 commit comments

Comments
 (0)