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

Minor optimizations #3687

Merged
merged 6 commits into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
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
11 changes: 9 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,8 @@ public class JobQueue<T> : IDisposable
/// </summary>
private readonly Action<string> _exceptionLogger;

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 +130,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 +157,7 @@ public void Pause()
CheckDisposed();

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

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

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

/// <summary>
Expand Down Expand Up @@ -275,8 +279,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)
Copy link
Member Author

Choose a reason for hiding this comment

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

Seemed to work okay, but not worth the risk of breaking this in production.

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)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
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);
Copy link
Member Author

Choose a reason for hiding this comment

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

This did a lot in the the method call. This way it is much easier to see what is passed on.

var discovererEnumerator = new DiscovererEnumerator(_requestData, discoveryResultCache, _cancellationTokenSource.Token);
discovererEnumerator.LoadTests(verifiedExtensionSourceMap, runSettings, discoveryCriteria.TestCaseFilter, _sessionMessageLogger);
}
}
finally
Expand Down