davkean
commented
7 years ago VS is basically unusable opening this solution.
Closed davkean closed 7 years ago
davkean
commented
7 years ago VS is basically unusable opening this solution.
lifengl
commented
7 years ago the problem here is that JTF library doesn't scale very well, if you have a JoinableTaskCollection, and its member JTF tasks join the collection itself. It basically creates lots of loops in the dependency graph, and causes the JTF to run into a slow code path. That will be O(n^3) time to close one JTF task (n is the number of tasks in the collection.)
We have a discussion email thread about this, and the code should be changed to:
Return this.threadingService.JoinableTaskFactory.RunAsync(async delegate
{
Using (JoinableCollection.Join())
Using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
Await JoinableFactory.RunAsync(() => task()).ConfigureAwait(false);
}
});or even remove the out JTF (more efficient, but with some risks)
Using (JoinableCollection.Join())
Using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
Await JoinableFactory.RunAsync(() => task()).ConfigureAwait(false);
}
AArnott
commented
7 years ago @lifengl Can you file a bug in the vs-threading repo with a repro of the N^3 algorithm so we can look at fixing it? Or at least updating the docs to prescribe the pattern that solves the 3rd party queue problem without incurring that n^3 overhead?
davkean
commented
7 years ago Is the extra func indirection actually needed? You also can't ConfigureAwait a JoinableTask, this is what I ended up with:
return JoinableFactory.RunAsync(async delegate
{
using (JoinableCollection.Join())
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
return await JoinableFactory.RunAsync(task);
}
});This didn't resolve the issue, @lifengl is the following an indicator of the issue?
davkean
commented
7 years ago This seems to have made things better, but performance is still horrible, and still lots of UI blocks, looking behind the scenes still lots of dependency node stuff.
private async Task<T> ExecuteWithinLockAsync<T>(Func<Task<T>> task)
{
using (JoinableCollection.Join())
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
return await JoinableFactory.RunAsync(task);
}
}You also can't ConfigureAwait a JoinableTask
Well, you can await a JoinableTask with ConfigureAwait(false) behavior, but we don't make it as easy as doing it for Task. In practice, folks who use JoinableTask don't tend to need to worry about .ConfigureAwait(false) so you're actually the first to have brought it up. :)
AArnott
commented
7 years ago Given the less aggressive change was insufficient, perhaps we should try @lifengl's more aggressive one:
using (JoinableCollection.Join())
{
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
await JoinableFactory.RunAsync(() => task()).Task.ConfigureAwait(false);
}
}This means task() will at least start on a background thread. The .ConfigureAwait(false) calls in this snippet ensure that the UI thread is not necessary to exit the gate after entering it, except for the actual task() work that is inside the JoinableTask. I'm assuming that JoinableFactory adds tasks to JoinableCollection. Provided that is true, then this snippet should be safe, and it should limit the number of JoinableTask instances in the collection to just 1 (or however many can enter the gate concurrently anyway), thereby solving the n^3 problem I believe.
davkean
commented
7 years ago Is it deliberate that you are creating an extra delegate in () => task()? I've just replaced it by passing task.
I missed that there was two ExecuteWithInLockAsync overloads, I'm fixing both and adding ConfigureAwait(false) to both. Performance is better in there's less UI blocks - but it's it's constantly chewing ~90 CPU, which I'm investigating.
lifengl
commented
7 years ago You can not use the same factory for the outside Joinable task factory, it must be a different one to break the loop. If you do, the two levels only make the problem worse.
You are right. The deep recursive is the indication of this performance problem. The JTF followed the loop dependency and you created more than 200 nodes each one depends on all others and 40000 arcs to scan.
Sent from my phone
On Jun 5, 2017, at 8:24 PM, David Kean notifications@github.com<mailto:notifications@github.com> wrote:
This didn't resolve the issue, @lifenglhttps://github.com/lifengl is this indicator of the issue:
[image]https://cloud.githubusercontent.com/assets/1103906/26812519/7c389a08-4abb-11e7-8cc1-8604e1ab6f87.png
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/project-system/issues/2383#issuecomment-306371231, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALGWwiHOeIWZu4ev6OYPNUM525hDS5F8ks5sBMZ6gaJpZM4NvvPb.
AArnott
commented
7 years ago Is it deliberate that you are creating an extra delegate in () => task()? I've just replaced it by passing task.
No. If task is a Func<Task> you can pass that in directly without the extra lambda.
davkean
commented
7 years ago Not having any luck reading the trace - would love some of your time to show me how to interpret data flow traces.
What's interesting is that the project has been opened for ~10 minutes, and I have 10 threads containing ProjectTreeProvider's hasn't even finished initializing:
mscorlib.dll!System.Threading.Monitor.Wait(object obj, int millisecondsTimeout, bool exitContext) Unknown
mscorlib.dll!System.Threading.Monitor.Wait(object obj, int millisecondsTimeout) Unknown
mscorlib.dll!System.Threading.ManualResetEventSlim.Wait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.SpinThenBlockingWait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.InternalWait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.Wait(int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) Unknown
mscorlib.dll!System.Threading.Tasks.Task.Wait(System.TimeSpan timeout) Unknown
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.WaitSynchronouslyCore(System.Threading.Tasks.Task task) Unknown
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.WaitSynchronously(System.Threading.Tasks.Task task) Unknown
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTask.CompleteOnCurrentThread() Unknown
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.Run(System.Func<System.Threading.Tasks.Task> asyncMethod) Unknown
> Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.DependenciesProjectTreeProvider.GetRule(Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.IDependency dependency, Microsoft.VisualStudio.ProjectSystem.Properties.IProjectCatalogSnapshot catalogs) Line 577 C#
Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.GroupedByTargetTreeViewProvider.CreateOrUpdateNode(Microsoft.VisualStudio.ProjectSystem.IProjectTree node, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.IDependency dependency, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.ITargetedDependenciesSnapshot targetedSnapshot, Microsoft.VisualStudio.ProjectSystem.Properties.IProjectCatalogSnapshot catalogs, bool isProjectItem, Microsoft.VisualStudio.ProjectSystem.ProjectTreeFlags? additionalFlags, Microsoft.VisualStudio.ProjectSystem.ProjectTreeFlags? excludedFlags) Line 347 C#
Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.GroupedByTargetTreeViewProvider.BuildSubTree(Microsoft.VisualStudio.ProjectSystem.IProjectTree rootNode, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.ITargetedDependenciesSnapshot targetedSnapshot, System.Collections.Generic.IEnumerable<Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.IDependency> dependencies, Microsoft.VisualStudio.ProjectSystem.Properties.IProjectCatalogSnapshot catalogs, bool isActiveTarget, bool shouldCleanup) Line 298 C#
Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.GroupedByTargetTreeViewProvider.BuildSubTrees(Microsoft.VisualStudio.ProjectSystem.IProjectTree rootNode, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.CrossTarget.ITargetFramework activeTarget, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.ITargetedDependenciesSnapshot targetedSnapshot, Microsoft.VisualStudio.ProjectSystem.Properties.IProjectCatalogSnapshot catalogs, System.Func<Microsoft.VisualStudio.ProjectSystem.IProjectTree, System.Collections.Generic.IEnumerable<Microsoft.VisualStudio.ProjectSystem.IProjectTree>, Microsoft.VisualStudio.ProjectSystem.IProjectTree> syncFunc) Line 244 C#
Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.GroupedByTargetTreeViewProvider.BuildTree(Microsoft.VisualStudio.ProjectSystem.IProjectTree dependenciesTree, Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.Snapshot.IDependenciesSnapshot snapshot, System.Threading.CancellationToken cancellationToken) Line 68 C#
Microsoft.VisualStudio.ProjectSystem.Managed.VS.dll!Microsoft.VisualStudio.ProjectSystem.VS.Tree.Dependencies.DependenciesProjectTreeProvider.BuildTreeForSnapshotAsync.AnonymousMethod__0(Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot> treeSnapshot, Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.ConfiguredProjectExports configuredProjectExports, System.Threading.CancellationToken cancellationToken) Line 398 C#
Microsoft.VisualStudio.ProjectSystem.dll!Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission.UpdateTree(Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot> currentTree, Microsoft.VisualStudio.ProjectSystem.CoreProjectTreeProviderBase.CoreConfiguredProjectExports configuredProjectExports, System.Threading.CancellationToken cancellationToken) Line 501 C#
Microsoft.VisualStudio.ProjectSystem.dll!Microsoft.VisualStudio.ProjectSystem.CoreProjectTreeProviderBase.Initialize.AnonymousMethod__1() Line 341 C#
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.ExecuteJob<System.Tuple<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>, System.Threading.Tasks.TaskCompletionSource<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>>>>(System.Func<System.Threading.Tasks.Task> asyncMethod, Microsoft.VisualStudio.Threading.JoinableTask job) Unknown
Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.RunAsync<System.Tuple<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>, System.Threading.Tasks.TaskCompletionSource<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>>>>(System.Func<System.Threading.Tasks.Task<System.Tuple<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>, System.Threading.Tasks.TaskCompletionSource<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>>>>> asyncMethod, bool synchronouslyBlocking, Microsoft.VisualStudio.Threading.JoinableTaskCreationOptions creationOptions) Unknown
Microsoft.VisualStudio.ProjectSystem.dll!Microsoft.VisualStudio.ProjectSystem.CoreProjectTreeProviderBase.Initialize.AnonymousMethod__0(Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission submission) Line 329 C#
System.Threading.Tasks.Dataflow.dll!System.Threading.Tasks.Dataflow.TransformBlock<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission, System.Tuple<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>, System.Threading.Tasks.TaskCompletionSource<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>>>>.ProcessMessageWithTask(System.Func<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission, System.Threading.Tasks.Task<System.Tuple<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>, System.Threading.Tasks.TaskCompletionSource<Microsoft.VisualStudio.ProjectSystem.IProjectVersionedValue<Microsoft.VisualStudio.ProjectSystem.IProjectTreeSnapshot>>>>> transform, System.Collections.Generic.KeyValuePair<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission, long> messageWithId) Unknown
System.Threading.Tasks.Dataflow.dll!System.Threading.Tasks.Dataflow.TransformBlock<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission, System.__Canon>..ctor.AnonymousMethod__4(System.Collections.Generic.KeyValuePair<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission, long> messageWithId) Unknown
System.Threading.Tasks.Dataflow.dll!System.Threading.Tasks.Dataflow.Internal.TargetCore<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission>.ProcessMessagesLoopCore() Unknown
System.Threading.Tasks.Dataflow.dll!System.Threading.Tasks.Dataflow.Internal.TargetCore<Microsoft.VisualStudio.ProjectSystem.ProjectTreeProviderBase.TreeUpdateSubmission>.ProcessAsyncIfNecessary_Slow.AnonymousMethod__3(object thisTargetCore) Unknown
Basically stuck here:
public IRule GetRule(IDependency dependency, IProjectCatalogSnapshot catalogs)
{
Requires.NotNull(dependency, nameof(dependency));
ConfiguredProject project = null;
if (dependency.TargetFramework.Equals(TargetFramework.Any))
{
project = ActiveConfiguredProject;
}
else
{
ThreadHelper.JoinableTaskFactory.Run(async () => <-- BLOCKED
{
project = await DependenciesHost.GetConfiguredProject(dependency.TargetFramework)
.ConfigureAwait(false) ?? ActiveConfiguredProject;
});
}This just addressed one issue -- the JTF overhead, but we still need take a look why the dependence tree generates so many calls. Can we queue them so we don't have so many threads to run them, or maybe pending requests can be merged so they can be processed more effectively, or old request might be expired when newer version data comes in, so that request can be dropped?
Sent from my phone
On Jun 5, 2017, at 8:42 PM, David Kean notifications@github.com<mailto:notifications@github.com> wrote:
This seems to have made things better, but performence is still horrible, and still lots of UI blocks, looking behind the scenes still lots of dependency node stuff.
private async Task<T> ExecuteWithinLockAsync<T>(Func<Task<T>> task)
{
using (JoinableCollection.Join())
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
return await JoinableFactory.RunAsync(task);
}
}— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/project-system/issues/2383#issuecomment-306373375, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALGWwkUDbPwC3Tq551uqGyRAQ46zTjrSks5sBMqlgaJpZM4NvvPb.
lifengl
commented
7 years ago My point is that JTF.Run should not be used in the thread pool code. It should be used only to implement old sync APIs, especially old COM contracts.
New contracts should follow asynchronous pattern, and we should never use JTF.Run in pure new code.
JTF.Run can cause thread pool exhaustion issues, especially when the task may take some time or require some resources/lock to finish and may be blocked for a long time.
Sent from my phone
On Jun 5, 2017, at 10:15 PM, David Kean notifications@github.com<mailto:notifications@github.com> wrote:
Basically stuck here:
public IRule GetRule(IDependency dependency, IProjectCatalogSnapshot catalogs) { Requires.NotNull(dependency, nameof(dependency));
ConfiguredProject project = null;
if (dependency.TargetFramework.Equals(TargetFramework.Any))
{
project = ActiveConfiguredProject;
}
else
{
ThreadHelper.JoinableTaskFactory.Run(async () => <-- BLOCKED
{
project = await DependenciesHost.GetConfiguredProject(dependency.TargetFramework)
.ConfigureAwait(false) ?? ActiveConfiguredProject;
});
}— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/project-system/issues/2383#issuecomment-306384139, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALGWwnyNA0mEqbqyqLG2F0jFuZDj9mHtks5sBOBdgaJpZM4NvvPb.
davkean
commented
7 years ago Yep...already fixing this, this is about 20 layers deep though, so it's a slog to mark it async.
AArnott
commented
7 years ago Yep...already fixing this, this is about 20 layers deep though, so it's a slog to mark it async.
I'm actually in process of writing a Roslyn code fix provider to automate this. :)
JTF.Run can cause thread pool exhaustion issues, especially when the task may take some time or require some resources/lock to finish and may be blocked for a long time.
I don't know of any faults in JTF.Run that lead to threadpool exhaustion over alternative methods like Task.Wait(). Rather, JTF.Run is more efficient when blocking a threadpool thread than Task.Wait() is because continuations can use the blocking thread. But if you're just saying that JTF.Run is bad for threadpool exhaustion simply by virtue of blocking a thread, I agree. And I agree: new code that calls async code should be written to be async itself rather than adding JTF.Run to that new code.
davkean
commented
7 years ago Okay, after replacing JTF.Run with async all the way down this made it much better, lot less threads and no threads "stuck" on that JTF.Run call. CPU is still pretty damn crazy elsewhere though - but that looks related to NuGet.
davkean
commented
7 years ago No, actually there's a tonne of work going on include dependency node - looks like everyone wants to run on idle.
lifengl
commented
7 years ago It is not a problem of JTF.Run. I agree that it may be more efficient than task.Wait. Basically any code blocking a thread pool for considerable time can cause thread pool exhaustion issues. That is worse when another thread pool task needed to finish the task to unblock it. It is just a problem that we tendered to use JTF.Run in the middle of asynchronous chains.
Sent from my phone
On Jun 5, 2017, at 11:03 PM, Andrew Arnott notifications@github.com<mailto:notifications@github.com> wrote:
Yep...already fixing this, this is about 20 layers deep though, so it's a slog to mark it async.
I'm actually in process of writing a Roslyn code fix provider to automate this. :)
JTF.Run can cause thread pool exhaustion issues, especially when the task may take some time or require some resources/lock to finish and may be blocked for a long time.
I don't know of any faults in JTF.Run that lead to threadpool exhaustion over alternative methods like Task.Wait(). Rather, JTF.Run is more efficient when blocking a threadpool thread than Task.Wait() is because continuations can use the blocking thread. But if you're just saying that JTF.Run is bad for threadpool exhaustion simply by virtue of blocking a thread, I agree. And I agree: new code that calls async code should be written to be async itself rather than adding JTF.Run to that new code.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/project-system/issues/2383#issuecomment-306390534, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALGWwndiU88ivVBg0YKYqCB75yuXLk7Kks5sBOucgaJpZM4NvvPb.
davkean
commented
7 years ago I filed https://github.com/dotnet/project-system/issues/2389 to track this JTF.Run usage.
This appears to be regression in 15.3 as these are all new paths:
Expected: For projects to open asynchronously and not block UI thread Actual: UI thread is blocked for > 10 minutes
Attaching I see there is 243 threads (!), the majority of which are populating or something to do with the dependency node:
UI thread:
44 threads:
22 threads:
74 threads:
47 threads: