Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions src/Microsoft.TestPlatform.Common/Utilities/SimpleJSON.cs
Original file line number Diff line number Diff line change
Expand Up @@ -856,16 +856,13 @@ public override JSONNode this[string aKey]
{
get
{
return _dict.ContainsKey(aKey) ? _dict[aKey] : new JSONLazyCreator(this, aKey);
return _dict.TryGetValue(aKey, out var value) ? value : new JSONLazyCreator(this, aKey);
}
set
{
if (value == null)
value = JSONNull.CreateOrGet();
if (_dict.ContainsKey(aKey))
_dict[aKey] = value;
else
_dict.Add(aKey, value);
_dict[aKey] = value;
}
}

Expand Down Expand Up @@ -898,20 +895,16 @@ public override void Add(string aKey, JSONNode aItem)

if (aKey != null)
{
if (_dict.ContainsKey(aKey))
_dict[aKey] = aItem;
else
_dict.Add(aKey, aItem);
_dict[aKey] = aItem;
}
else
_dict.Add(Guid.NewGuid().ToString(), aItem);
}

public override JSONNode Remove(string aKey)
{
if (!_dict.ContainsKey(aKey))
if (!_dict.TryGetValue(aKey, out var tmp))
return null;
JSONNode tmp = _dict[aKey];
_dict.Remove(aKey);
return tmp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,14 @@ public ITestRunStatistics GetAggregatedRunStats()
foreach (var runStats in _testRunStatsList)
{
// TODO: we get nullref here if the stats are empty.
foreach (var outcome in runStats.Stats!.Keys)
foreach (var kvp in runStats.Stats!)
{
if (!testOutcomeMap.ContainsKey(outcome))
if (!testOutcomeMap.TryGetValue(kvp.Key, out long currentCount))
{
testOutcomeMap.Add(outcome, 0);
currentCount = 0;
}
testOutcomeMap[outcome] += runStats.Stats[outcome];

testOutcomeMap[kvp.Key] = currentCount + kvp.Value;
}
totalTests += runStats.ExecutedTests;
}
Expand Down Expand Up @@ -198,9 +199,9 @@ public void AggregateRunDataMetrics(IDictionary<string, object>? metrics)
var newValue = Convert.ToDouble(metric.Value, CultureInfo.InvariantCulture);

_metricsAggregator.AddOrUpdate(
metric.Key,
newValue,
(_, oldValue) => newValue + Convert.ToDouble(oldValue, CultureInfo.InvariantCulture));
metric.Key,
newValue,
(_, oldValue) => newValue + Convert.ToDouble(oldValue, CultureInfo.InvariantCulture));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,12 @@ public IDictionary<string, string> GetDataCollectionDataSetForTestCase(Guid test

private void AddKeyValuePairToDictionary(Guid testCaseId, string key, string value)
{
if (!_testCaseDataCollectionDataMap.ContainsKey(testCaseId))
if (!_testCaseDataCollectionDataMap.TryGetValue(testCaseId, out var testCaseCollectionData))
{
var testCaseCollectionData = new TestCaseDataCollectionData();
testCaseCollectionData.AddOrUpdateData(key, value);
testCaseCollectionData = new TestCaseDataCollectionData();
_testCaseDataCollectionDataMap[testCaseId] = testCaseCollectionData;
}
else
{
_testCaseDataCollectionDataMap[testCaseId].AddOrUpdateData(key, value);
}
testCaseCollectionData.AddOrUpdateData(key, value);
}

private class TestCaseDataCollectionData
Expand All @@ -76,15 +72,11 @@ public TestCaseDataCollectionData()

internal void AddOrUpdateData(string key, string value)
{
if (!CollectionData.ContainsKey(key))
{
CollectionData[key] = value;
}
else
if (CollectionData.ContainsKey(key))
{
EqtTrace.Warning("The data for in-proc data collector with key {0} has already been set. Will be reset with new value", key);
CollectionData[key] = value;
}
CollectionData[key] = value;
Comment on lines +75 to +79
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path still does a double-lookup (ContainsKey then indexer assignment). Since this PR targets dictionary lookup reductions, consider switching to TryAdd (warn+overwrite if it returns false) or TryGetValue to keep the warning behavior while avoiding an extra hash lookup on the common add path.

Copilot uses AI. Check for mistakes.
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Threading;

using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
Expand Down Expand Up @@ -67,7 +69,7 @@ public override void HandleTestRunComplete(
_runDataAggregator.IsAborted,
_runDataAggregator.GetAggregatedException(),
_runDataAggregator.RunContextAttachments,
_runDataAggregator.InvokedDataCollectors,
new Collection<InvokedDataCollector>(_runDataAggregator.InvokedDataCollectors.ToList()),
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This materializes a new List<T> via ToList() and then wraps it in a new Collection<T>, adding allocations on the completion path. If the downstream API can be adjusted, prefer accepting IReadOnlyCollection<T> / IEnumerable<T> to avoid forcing a copy. If the signature cannot change, consider passing a one-time snapshot that is created earlier (or reusing a cached snapshot) so this call site doesn't allocate on every completion.

Suggested change
new Collection<InvokedDataCollector>(_runDataAggregator.InvokedDataCollectors.ToList()),
_runDataAggregator.InvokedDataCollectors.ToList(),

Copilot uses AI. Check for mistakes.
_runDataAggregator.ElapsedTime);

// Add Metrics from Test Host
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ public virtual ProxyOperationManager DequeueProxy(string source, string? runSett
lock (_proxyOperationLockObject)
{
// No proxy available means the caller will have to create its own proxy.
if (!_proxyMap.ContainsKey(source)
|| !_proxyContainerList[_proxyMap[source]].IsAvailable)
if (!_proxyMap.TryGetValue(source, out int proxyIndex)
|| !_proxyContainerList[proxyIndex].IsAvailable)
{
throw new InvalidOperationException(CrossPlatResources.NoAvailableProxyForDeque);
}
Expand All @@ -273,7 +273,7 @@ public virtual ProxyOperationManager DequeueProxy(string source, string? runSett
}

// Get the actual proxy.
proxyContainer = _proxyContainerList[_proxyMap[source]];
proxyContainer = _proxyContainerList[proxyIndex];

// Mark the proxy as unavailable.
proxyContainer.IsAvailable = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ public virtual bool KillSession(TestSessionInfo testSessionInfo, IRequestData re
lock (_lockObject)
{
// Check if the session info exists.
if (!_sessionPool.ContainsKey(testSessionInfo))
if (!_sessionPool.TryGetValue(testSessionInfo, out var proxyManagerFromPool))
{
return false;
}

// Remove the session from the pool.
proxyManager = _sessionPool[testSessionInfo];
proxyManager = proxyManagerFromPool;
_sessionPool.Remove(testSessionInfo);
}

Expand Down Expand Up @@ -136,13 +136,10 @@ public virtual bool KillSession(TestSessionInfo testSessionInfo, IRequestData re
ProxyTestSessionManager? sessionManager;
lock (_lockObject)
{
if (!_sessionPool.ContainsKey(testSessionInfo))
if (!_sessionPool.TryGetValue(testSessionInfo, out sessionManager))
{
return null;
}

// Gets the session manager reference from the pool.
sessionManager = _sessionPool[testSessionInfo];
}

try
Expand Down Expand Up @@ -184,13 +181,10 @@ public virtual bool ReturnProxy(TestSessionInfo testSessionInfo, int proxyId)
ProxyTestSessionManager? sessionManager;
lock (_lockObject)
{
if (!_sessionPool.ContainsKey(testSessionInfo))
if (!_sessionPool.TryGetValue(testSessionInfo, out sessionManager))
{
return false;
}

// Gets the session manager reference from the pool.
sessionManager = _sessionPool[testSessionInfo];
}

try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public static string ToCommandLineOptions(this TestRunnerConnectionInfo connecti
var options = $"--port {connectionInfo.Port} --endpoint {connectionInfo.ConnectionInfo.Endpoint} --role {(connectionInfo.ConnectionInfo.Role == ConnectionRole.Client ? "client" : "host")} --parentprocessid {connectionInfo.RunnerProcessId}";
if (!StringUtils.IsNullOrEmpty(connectionInfo.LogFile))
{
options += " --diag " + connectionInfo.LogFile;
options += " --tracelevel " + connectionInfo.TraceLevel;
options = $"{options} --diag {connectionInfo.LogFile} --tracelevel {connectionInfo.TraceLevel}";
}

return options;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public SessionStartEventArgs(DataCollectionContext context, IDictionary<string,
{
ValidateArg.NotNullOrEmpty(property, nameof(property));

return _properties.ContainsKey(property) ? (T?)_properties[property] : default;
return _properties.TryGetValue(property, out var propertyValue) ? (T?)propertyValue : default;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public TestSessionStartArgs(string configuration)
{
ValidateArg.NotNullOrEmpty(property, nameof(property));
TPDebug.Assert(_properties is not null, "_properties is null");
return _properties.ContainsKey(property) ? (T?)_properties[property] : default;
return _properties.TryGetValue(property, out var propertyValue) ? (T?)propertyValue : default;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,9 @@ private void PopulateCacheForTypeAndMethodSymbols()
try
{
typeName = typeName.Replace('+', '.');
if (_typeSymbols.ContainsKey(typeName))
if (_typeSymbols.TryGetValue(typeName, out var cachedSymbol))
{
return _typeSymbols[typeName];
return cachedSymbol;
}

TPDebug.Assert(_session is not null, "_session is null");
Expand Down Expand Up @@ -384,12 +384,12 @@ private void PopulateCacheForTypeAndMethodSymbols()
try
{
typeSymbol.GetName(out string symbolName);
if (_methodSymbols.ContainsKey(symbolName))
if (_methodSymbols.TryGetValue(symbolName, out var existingMethodSymbols))
{
methodSymbolsForType = _methodSymbols[symbolName];
if (methodSymbolsForType.ContainsKey(methodName))
methodSymbolsForType = existingMethodSymbols;
if (methodSymbolsForType.TryGetValue(methodName, out var cachedMethodSymbol))
{
return methodSymbolsForType[methodName];
return cachedMethodSymbol;
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,13 @@ public void Dispose()
/// </returns>
public INavigationData? GetNavigationData(string declaringTypeName, string methodName)
{
INavigationData? navigationData = null;
if (_methodsNavigationDataForType.ContainsKey(declaringTypeName))
if (_methodsNavigationDataForType.TryGetValue(declaringTypeName, out var methodDict)
&& methodDict.TryGetValue(methodName, out var navigationData))
{
var methodDict = _methodsNavigationDataForType[declaringTypeName];
if (methodDict.ContainsKey(methodName))
{
navigationData = methodDict[methodName];
}
return navigationData;
}

return navigationData;
return null;
}

/// <summary>
Expand Down
10 changes: 8 additions & 2 deletions src/vstest.console/Processors/EnableBlameArgumentProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@

namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Processors;

static file class BlameParameterNames
{
public static HashSet<string> CrashDumpKeys { get; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "CollectAlways", "DumpType" };
public static HashSet<string> HangDumpKeys { get; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "TestTimeout", "HangDumpType" };
}

internal class EnableBlameArgumentProcessor : IArgumentProcessor
{
/// <summary>
Expand Down Expand Up @@ -208,7 +214,7 @@ private void InitializeBlame(bool enableCrashDump, bool enableHangDump, bool mon
if (enableCrashDump)
{
var dumpParameters = collectDumpParameters
?.Where(p => new[] { "CollectAlways", "DumpType" }.Contains(p.Key, StringComparer.OrdinalIgnoreCase))
?.Where(p => BlameParameterNames.CrashDumpKeys.Contains(p.Key))
.ToDictionary(p => p.Key, p => p.Value, StringComparer.OrdinalIgnoreCase)
?? new Dictionary<string, string>();

Expand All @@ -224,7 +230,7 @@ private void InitializeBlame(bool enableCrashDump, bool enableHangDump, bool mon
if (enableHangDump)
{
var hangDumpParameters = collectDumpParameters
?.Where(p => new[] { "TestTimeout", "HangDumpType" }.Contains(p.Key, StringComparer.OrdinalIgnoreCase))
?.Where(p => BlameParameterNames.HangDumpKeys.Contains(p.Key))
.ToDictionary(p => p.Key, p => p.Value, StringComparer.OrdinalIgnoreCase)
?? new Dictionary<string, string>();

Expand Down
Loading