Skip to content

Fixing .net framework parallel issue #3192

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.

Sign up for GitHub

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

Merged
Sanan07 merged 1 commit into from
Nov 25, 2021
Merged

Fixing .net framework parallel issue #3192

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 18 additions & 19 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyDiscoveryManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.Parallel
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel;
Expand Down Expand Up @@ -145,27 +144,27 @@ when testhost crashed by itself and when user requested it (f.e. through TW)
return true;
}

// Discovery is not complete.
// First, clean up the used proxy discovery manager if the last run was aborted
// or this run doesn't support shared hosts (netcore tests)
if (!this.SharedHosts || isAborted)
/* Discovery is not complete.
Now when both.net framework and.net core projects can run in parallel
we should clear manager and create new one for both cases.
Otherwise `proxyDiscoveryManager` instance is alredy closed by now and it will give exception
when trying to do some operation on it.
*/
if (EqtTrace.IsVerboseEnabled)
{
if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose("ParallelProxyDiscoveryManager: HandlePartialDiscoveryComplete: Replace discovery manager. Shared: {0}, Aborted: {1}.", this.SharedHosts, isAborted);
}
EqtTrace.Verbose("ParallelProxyDiscoveryManager: HandlePartialDiscoveryComplete: Replace discovery manager. Shared: {0}, Aborted: {1}.", this.SharedHosts, isAborted);
}

this.RemoveManager(proxyDiscoveryManager);
this.RemoveManager(proxyDiscoveryManager);

proxyDiscoveryManager = this.CreateNewConcurrentManager();
Copy link
Member

@cvpoienaru cvpoienaru Nov 25, 2021

Choose a reason for hiding this comment

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

We should probably take advantage of the fact that .net fw testhost is shared and re-use it instead of killing it in the parallel discovery/execution case.

E.g.: parallelLevel = 4, .net fw test assmeblies = 8.

What happens now is that we spawn 4 testhosts, discover 4 .net fw test assmeblies, kill the 4 testhosts and then spawn another 4 testhosts to do the same for the remaining 4 test assemblies.

Copy link
Contributor Author

@Sanan07 Sanan07 Nov 25, 2021

Choose a reason for hiding this comment

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

Actually, after discovery 4 test assemblies, it will spawn next 4 testhosts one by one, not simultaniously as the first time.
So it will spawn 4 testhosts and when some testhost finishes discovering it will start new one for the next source.

As we discussed, I just created new issue for that idea : #3193.
Feel free to add or edit description for the issue.

var parallelEventsHandler = new ParallelDiscoveryEventsHandler(
this.requestData,
proxyDiscoveryManager,
this.currentDiscoveryEventsHandler,
this,
this.currentDiscoveryDataAggregator);
this.AddManager(proxyDiscoveryManager, parallelEventsHandler);
}
proxyDiscoveryManager = this.CreateNewConcurrentManager();
var parallelEventsHandler = new ParallelDiscoveryEventsHandler(
this.requestData,
proxyDiscoveryManager,
this.currentDiscoveryEventsHandler,
this,
this.currentDiscoveryDataAggregator);
this.AddManager(proxyDiscoveryManager, parallelEventsHandler);

// Second, let's attempt to trigger discovery for the next source.
this.DiscoverTestsOnConcurrentManager(proxyDiscoveryManager);
Expand Down