Skip to content

Commit

Permalink
Minor optimizations (#3687)
Browse files Browse the repository at this point in the history
* Minor performance optimizations

* Rename contractResolver variable

* Revert handlers

* Remove backups

* Build?
  • Loading branch information
nohwnd authored Jun 2, 2022
1 parent 3f1f124 commit ce79eb4
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 21 deletions.
4 changes: 0 additions & 4 deletions TestPlatform.sln
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "testhost.arm64", "src\testh
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DumpMinitool.arm64", "src\DataCollectors\DumpMinitool.arm64\DumpMinitool.arm64.csproj", "{62E9D32B-B989-43CF-81A2-B38B3367FCA3}"
EndProject
Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Microsoft.TestPlatform.Nullability", "src\Microsoft.TestPlatform.Nullability\Microsoft.TestPlatform.Nullability.shproj", "{9DF3BC3C-2A53-46E7-9291-40728ACE5F82}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -1025,7 +1023,6 @@ Global
{29270853-90DC-4C39-9621-F47AE40A79B6} = {B27FAFDF-2DBA-4AB0-BA85-FD5F21D359D6}
{186069FE-E1E8-4DE1-BEA4-0FF1484D22D1} = {ED0C35EB-7F31-4841-A24F-8EB708FFA959}
{62E9D32B-B989-43CF-81A2-B38B3367FCA3} = {B705537C-B82C-4A30-AFA5-6244D9A7DAEB}
{9DF3BC3C-2A53-46E7-9291-40728ACE5F82} = {7D4082EA-7AC9-4DFB-98E8-C5E08BDC0EC3}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {0541B30C-FF51-4E28-B172-83F5F3934BCD}
Expand All @@ -1039,7 +1036,6 @@ Global
src\Microsoft.TestPlatform.Nullability\Microsoft.TestPlatform.Nullability.projitems*{68adc720-316e-4895-9f8e-c3ccadd262be}*SharedItemsImports = 5
src\Microsoft.TestPlatform.Execution.Shared\Microsoft.TestPlatform.Execution.Shared.projitems*{71cb42ff-e750-4a3b-9c3a-ac938853cc89}*SharedItemsImports = 5
src\Microsoft.TestPlatform.Execution.Shared\Microsoft.TestPlatform.Execution.Shared.projitems*{7f26eda3-c8c4-4b7f-a9b6-d278c2f40a13}*SharedItemsImports = 13
src\Microsoft.TestPlatform.Nullability\Microsoft.TestPlatform.Nullability.projitems*{9df3bc3c-2a53-46e7-9291-40728ace5f82}*SharedItemsImports = 13
src\Microsoft.TestPlatform.Nullability\Microsoft.TestPlatform.Nullability.projitems*{fbf74c8f-695c-4967-84ac-358eefb1376d}*SharedItemsImports = 5
EndGlobalSection
EndGlobal
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,25 @@ private void TestRunMessageHandler(object sender, TestRunMessageEventArgs e)
/// </summary>
private void SafeInvokeAsync(Func<MulticastDelegate> eventHandlersFactory, EventArgs args, int size, string traceDisplayName)
{
// If you are wondering why this is taking a Func<MulticastDelegate> rather than just a MulticastDelegate it is because
// taking just that will capture only the subscribers that were present at the time we passed the delegate into this
// method. Which means that if there were no subscribers we will capture null, and later when the queue is unpaused
// we won't call any logger that subscribed while the queue was paused.

ValidateArg.NotNull(eventHandlersFactory, nameof(eventHandlersFactory));
ValidateArg.NotNull(args, nameof(args));

// When the logger event queue is paused we want to enqueue the work because maybe there are no
// loggers yet, and there are maybe errors that the loggers should report once they subscribe.
// When the queue is running, don't bother adding tasks to the queue when there are no subscribers
// because there is very slim chance that a new logger will be added in the few milliseconds that will
// pass until we process the task, and it just puts pressure on the queue. If this proves to be a problem
// we have a race condition, and that side should pause the queue while it adds all the loggers and then resume.
if (!_loggerEventQueue.IsPaused && eventHandlersFactory() == null)
{
return;
}

// Invoke the handlers on a background thread.
_loggerEventQueue.QueueJob(
() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ private JsonDataSerializer()
s_payloadSerializer = JsonSerializer.Create(jsonSettings);
s_payloadSerializer2 = JsonSerializer.Create(jsonSettings);

var contractResolver = new DefaultTestPlatformContractResolver();
s_fastJsonSettings = new JsonSerializerSettings
{
DateFormatHandling = jsonSettings.DateFormatHandling,
Expand All @@ -61,11 +62,11 @@ private JsonDataSerializer()
// of changing how our consumers get their data.
// NullValueHandling = NullValueHandling.Ignore,

ContractResolver = new DefaultTestPlatformContractResolver(),
ContractResolver = contractResolver,
};

s_payloadSerializer.ContractResolver = new TestPlatformContractResolver1();
s_payloadSerializer2.ContractResolver = new DefaultTestPlatformContractResolver();
s_payloadSerializer2.ContractResolver = contractResolver;

#if TRACE_JSON_SERIALIZATION
// MemoryTraceWriter can help diagnose serialization issues. Enable it for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,4 @@ static Microsoft.VisualStudio.TestPlatform.Utilities.TimeSpanParser.Parse(string
static Microsoft.VisualStudio.TestPlatform.Utilities.TimeSpanParser.TryParse(string time, out System.TimeSpan result) -> bool
virtual Microsoft.VisualStudio.TestPlatform.Utilities.JobQueue<T>.WaitForQueueToGetEmpty() -> bool
static Microsoft.VisualStudio.TestPlatform.Utilities.MulticastDelegateUtilities.SafeInvoke(this System.Delegate delegates, object sender, object args, string traceDisplayName) -> void
Microsoft.VisualStudio.TestPlatform.Utilities.JobQueue<T>.IsPaused.get -> bool
15 changes: 13 additions & 2 deletions src/Microsoft.TestPlatform.CoreUtilities/Utilities/JobQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ public class JobQueue<T> : IDisposable
/// </summary>
private readonly Action<string> _exceptionLogger;

/// <summary>
/// True when the job queue is paused. Don't use this for synchronization,
/// it is not super thread-safe. Just use it to see if the queue was started already.
/// </summary>
public bool IsPaused { get; private set; }

/// <summary>
/// Initializes a new instance of the <see cref="JobQueue{T}"/> class.
/// </summary>
Expand Down Expand Up @@ -128,7 +134,7 @@ public JobQueue(Action<T> processJob, string displayName, int maxQueueLength, in
_exceptionLogger = exceptionLogger;

// Setup the background thread to process the jobs.
_backgroundJobProcessor = new Task(() => BackgroundJobProcessor(), TaskCreationOptions.LongRunning);
_backgroundJobProcessor = new Task(() => BackgroundJobProcessor(_displayName), TaskCreationOptions.LongRunning);
_backgroundJobProcessor.Start();
}

Expand All @@ -155,6 +161,7 @@ public void Pause()
CheckDisposed();

// Do not allow any jobs to be processed.
IsPaused = true;
_queueProcessing.Reset();
}

Expand All @@ -167,6 +174,7 @@ public void Resume()

// Resume processing of jobs.
_queueProcessing.Set();
IsPaused = false;
}

/// <summary>
Expand Down Expand Up @@ -275,8 +283,11 @@ private void CheckDisposed()
/// <summary>
/// Method which processes the jobs on the background thread.
/// </summary>
private void BackgroundJobProcessor()
private void BackgroundJobProcessor(string threadName)
{
#if DEBUG && (NETFRAMEWORK || NET || NETSTANDARD2_0_OR_GREATER)
Thread.CurrentThread.Name = threadName;
#endif
bool shutdown = false;

do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,22 @@ public static void SafeInvoke(this Delegate delegates, object sender, object arg
try
{
handler.DynamicInvoke(sender, args);
EqtTrace.Verbose("MulticastDelegateUtilities.SafeInvoke: {0}: Invoking callback {1}/{2} for {3}.{4}, took {5} ms.",
traceDisplayName,
++i,
invocationList.Length,
handler.GetTargetName(),
handler.GetMethodName(),
stopwatch.ElapsedMilliseconds);
if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose("MulticastDelegateUtilities.SafeInvoke: {0}: Invoking callback {1}/{2} for {3}.{4}, took {5} ms.",
traceDisplayName,
++i,
invocationList.Length,
handler.GetTargetName(),
handler.GetMethodName(),
stopwatch.ElapsedMilliseconds);
}
}
catch (TargetInvocationException exception)
{
EqtTrace.Error(
if (EqtTrace.IsErrorEnabled)
{
EqtTrace.Error(
"MulticastDelegateUtilities.SafeInvoke: {0}: Invoking callback {1}/{2} for {3}.{4}, failed after {5} ms with: {6}.",
++i,
invocationList.Length,
Expand All @@ -78,6 +83,7 @@ public static void SafeInvoke(this Delegate delegates, object sender, object arg
traceDisplayName,
stopwatch.ElapsedMilliseconds,
exception);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,9 @@ public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEve
// If there are sources to discover
if (verifiedExtensionSourceMap.Any())
{
new DiscovererEnumerator(_requestData, discoveryResultCache, _cancellationTokenSource.Token).LoadTests(
verifiedExtensionSourceMap,
RunSettingsUtilities.CreateAndInitializeRunSettings(discoveryCriteria.RunSettings),
discoveryCriteria.TestCaseFilter,
_sessionMessageLogger);
var runSettings = RunSettingsUtilities.CreateAndInitializeRunSettings(discoveryCriteria.RunSettings);
var discovererEnumerator = new DiscovererEnumerator(_requestData, discoveryResultCache, _cancellationTokenSource.Token);
discovererEnumerator.LoadTests(verifiedExtensionSourceMap, runSettings, discoveryCriteria.TestCaseFilter, _sessionMessageLogger);
}
}
finally
Expand Down

0 comments on commit ce79eb4

Please sign in to comment.