[Responsiveness] Roslyn initialization is forced onto the UI thread, forcing the project system to synchronize initialize it (6.9% of solution open) #55297

davkean · 2016-08-11T01:27:14Z

IWorkspaceProjectContextFactory.CreateProjectContextAsync is forced on the UI thread via Roslyn during solution load. UI thread is scarce resource during solution load and this means that Roslyn is prevented from being initialized until each project synchronously blocks on it in PrioritizedOnAfterOpenProject.

The project system would like to overlap this initialization so that we can make use of as many cores as possible during load.

davkean · 2016-09-15T21:00:29Z

We cannot do this, until we've fixed the race in dotnet/project-system#485.

mavasani · 2016-12-14T19:11:18Z

This is now blocked by #15871

Pilchie · 2017-12-19T20:09:35Z

Taking this, since @jasonmalinowski's work will likely not be ready before @davkean is out.

Pilchie · 2018-01-18T23:26:57Z

Moving to 15.7, since this work is risky and we'll try to tackle this early in 15.7.

davkean · 2018-11-07T02:11:43Z

Everything but creation is thread-safe and thread-agnostic.

davkean · 2018-11-07T04:38:33Z

We have done this for population and dispose (dotnet/project-system#4229), and just waiting on creation. We are blocked on Roslyn making a change around this.

davkean · 2018-11-07T08:42:30Z

Dispose isn't free-threaded, so backed that out: dotnet/project-system#4234

    Microsoft.CodeAnalysis.Workspaces.dll!Roslyn.Utilities.Contract.ThrowIfFalse(bool condition, string message)    Unknown
    Microsoft.CodeAnalysis.EditorFeatures.dll!Microsoft.CodeAnalysis.Editor.Shared.Utilities.ForegroundThreadAffinitizedObject.AssertIsForeground()    Unknown
    Microsoft.VisualStudio.LanguageServices.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.SetDocumentContext(Microsoft.CodeAnalysis.DocumentId documentId)    Unknown
    Microsoft.CodeAnalysis.Workspaces.dll!Microsoft.CodeAnalysis.Workspace.ClearOpenDocument(Microsoft.CodeAnalysis.DocumentId documentId, bool isSolutionClosing)    Unknown
    Microsoft.CodeAnalysis.Workspaces.dll!Microsoft.CodeAnalysis.Workspace.ClearOpenDocuments(Microsoft.CodeAnalysis.ProjectId projectId)    Unknown
    Microsoft.CodeAnalysis.Workspaces.dll!Microsoft.CodeAnalysis.Workspace.ClearProjectData(Microsoft.CodeAnalysis.ProjectId projectId)    Unknown
    Microsoft.CodeAnalysis.Workspaces.dll!Microsoft.CodeAnalysis.Workspace.OnProjectRemoved(Microsoft.CodeAnalysis.ProjectId projectId)    Unknown
    Microsoft.VisualStudio.LanguageServices.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.OnProjectRemoved(Microsoft.CodeAnalysis.ProjectId projectId)    Unknown
    Microsoft.VisualStudio.LanguageServices.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioProject.RemoveFromWorkspace.AnonymousMethod__86_0(Microsoft.CodeAnalysis.Workspace w)    Unknown
    Microsoft.VisualStudio.LanguageServices.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.ApplyChangeToWorkspace(System.Action<Microsoft.CodeAnalysis.Workspace> action)    Unknown
    Microsoft.VisualStudio.LanguageServices.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioProject.RemoveFromWorkspace()    Unknown
    Microsoft.VisualStudio.LanguageServices.Implementation.dll!Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.CPS.CPSProject.Dispose()    Unknown

drewnoakes · 2021-02-01T08:47:36Z

We still have two instances of switching to the UI thread in WorkspaceProjectContextProvider that reference this issue. Re-opening.

tmeschter · 2021-02-02T19:00:43Z

@drewnoakes Is the work solely on our side, or do we need something further from Roslyn to eliminate those last two places we switch to the UI thread?

davkean · 2021-02-02T19:18:10Z

Roslyn needs work.

drewnoakes · 2021-02-05T00:47:36Z

Filed #51016 to track the Roslyn side of this work.

drewnoakes · 2021-02-10T22:01:30Z

dotnet/project-system#6953 removes one of the two UI switches.

drewnoakes · 2021-02-12T12:28:03Z

#51138 is the Roslyn-side of the other UI switch.

CyrusNajmabadi · 2021-02-19T01:43:10Z

The roslyn side of this is complete. You can go ahead with this @drewnoakes

drewnoakes · 2021-02-19T03:02:31Z

Thanks @CyrusNajmabadi. I'll pick this up once an updated version of the Microsoft.VisualStudio.LanguageServices package is available. The latest seems to be from 8 days ago, unless I'm looking at the wrong feed.

drewnoakes · 2021-02-19T03:05:39Z

Oh wait, I see the feed to use: https://dev.azure.com/azure-public/vside/_packaging?_a=package&feed=vssdk&package=Microsoft.VisualStudio.LanguageServices&protocolType=NuGet&version=3.10.0-2.21118.48

Fixes #353 Roslyn recently implemented a free-threaded version of this API, meaning we no longer need to switch to the UI thread before calling it. This commit bumps the package version, removes the thread switch and calls the new API.

drewnoakes · 2021-05-12T07:28:44Z

Re-opening. Roslyn made changes to support this, but it introduced DDRIT failures.

cc @jasonmalinowski @CyrusNajmabadi

Fixes: .NET Project System's side of https://github.com/dotnet/project-system/issues/353. Roslyn already immediately switches to the UI thread CreateProjectContextAsync, so this should be a no-op. However, this enables the above bug to be moved over to Roslyn and for them to remove the UI thread dependency in the future.

CyrusNajmabadi · 2021-07-30T17:06:53Z

That seems reasonable. Though i think @jasonmalinowski will need to be pulled on this. He has the most context on why parts of our PS impl are UI thread affinitized. I have no doubt we can get (almost) everything off the UI thread, but he'll be the best at just knowing the places that are currently problematic.

davkean · 2021-07-30T22:34:06Z

I've moved it over, the project system change has been merged in.

jasonmalinowski · 2021-09-13T23:50:51Z

@CyrusNajmabadi @davkean I'm confused for what the remaining asks are here for this bug. It looks like @drewnoakes reactivated it when @CyrusNajmabadi's fix had DDRIT failures, but it seems all the changes have now gone in?

davkean · 2021-09-13T23:55:30Z

Project System removed their switch but this path still immediately switches to the UI thread, blocking it and preventing other things from progressing and overlapping.

CyrusNajmabadi · 2021-09-14T18:44:23Z

Yes. The only work we did was to enable this to be called asynchronously from any thread. We didn't do anything on our end to make things better on the UI thread.

jasonmalinowski · 2022-03-22T21:29:54Z

@drewnoakes I think we can close this at this point, or do you still have a UI thread switch before calling into Roslyn somewhere?

davkean · 2022-03-22T21:31:33Z

@jasonmalinowski This is representing the time Roslyn spends on the UI thread, last I saw in some traces, this was up to 30%. This isn't resolved.

jasonmalinowski · 2022-03-22T21:32:30Z

@davkean Can you share the trace then?

jasonmalinowski · 2022-03-22T21:38:31Z

@davkean Also to be clear I'm using this bug to track the original request which was making the project initialization path moved off the UI thread which is now as far as it can go without having shell APIs changed; if there's other paths that aren't that, we should have bugs for those, but let's have new bugs for those.

davkean · 2022-03-22T21:49:33Z

@jasonmalinowski I filed the original bug, and its always been about Roslyn chewing the UI thread.

All Orchard Core traces are showing this. Here's from yesterday's build from a cached load:

If you believe that Roslyn are unable to address these, then dupe these against the shell bugs we've filed. Looking at the trace, ~~I see that Roslyn forces the editor package to be initialized on the UI thread, which is not helping.~~ This is editor's fault.

jasonmalinowski · 2022-03-22T21:55:03Z

@davkean Alright, we can take a look at fixing that, thanks for the info!

jasonmalinowski · 2022-03-22T21:56:51Z

(and indeed that's a different stack than I've seen be expensive before...)

srivatsn assigned davkean Oct 21, 2016

Pilchie assigned Pilchie and unassigned davkean Dec 19, 2017

davkean unassigned Pilchie Nov 7, 2018

jmarolf closed this as completed in dotnet/project-system@5f50054 Jan 4, 2019

drewnoakes reopened this Feb 1, 2021

drewnoakes self-assigned this Feb 5, 2021

This comment has been minimized.

Sign in to view

drewnoakes closed this as completed in dotnet/project-system#6979 Feb 22, 2021

davkean unassigned drewnoakes Jul 30, 2021

davkean transferred this issue from dotnet/project-system Jul 30, 2021

dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 30, 2021

jinujoseph added Area-Performance and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 31, 2021

jinujoseph assigned jasonmalinowski Jul 31, 2021

jinujoseph added the Concept-Continuous Improvement label Jul 31, 2021

jinujoseph added this to the 17.0 milestone Jul 31, 2021

jasonmalinowski modified the milestones: 17.0, 17.1 Sep 14, 2021

jasonmalinowski modified the milestones: 17.1, 17.2 Jan 5, 2022

jasonmalinowski mentioned this issue Mar 23, 2022

Speed up VisualStudioProject construction #60351

Merged

jasonmalinowski closed this as completed in #60351 Mar 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Responsiveness] Roslyn initialization is forced onto the UI thread, forcing the project system to synchronize initialize it (6.9% of solution open) #55297

[Responsiveness] Roslyn initialization is forced onto the UI thread, forcing the project system to synchronize initialize it (6.9% of solution open) #55297

davkean commented Aug 11, 2016 •

edited

Loading

davkean commented Sep 15, 2016

mavasani commented Dec 14, 2016

Pilchie commented Dec 19, 2017

Pilchie commented Jan 18, 2018

davkean commented Nov 7, 2018

davkean commented Nov 7, 2018

davkean commented Nov 7, 2018 •

edited

Loading

drewnoakes commented Feb 1, 2021 •

edited

Loading

tmeschter commented Feb 2, 2021

davkean commented Feb 2, 2021 via email •

edited by ghost

Loading

drewnoakes commented Feb 5, 2021

drewnoakes commented Feb 10, 2021

drewnoakes commented Feb 12, 2021

CyrusNajmabadi commented Feb 19, 2021

drewnoakes commented Feb 19, 2021

drewnoakes commented Feb 19, 2021

This comment has been minimized.

drewnoakes commented May 12, 2021

CyrusNajmabadi commented Jul 30, 2021

davkean commented Jul 30, 2021

jasonmalinowski commented Sep 13, 2021

davkean commented Sep 13, 2021 •

edited

Loading

CyrusNajmabadi commented Sep 14, 2021

jasonmalinowski commented Mar 22, 2022

davkean commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

davkean commented Mar 22, 2022 •

edited

Loading

jasonmalinowski commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

[Responsiveness] Roslyn initialization is forced onto the UI thread, forcing the project system to synchronize initialize it (6.9% of solution open) #55297

[Responsiveness] Roslyn initialization is forced onto the UI thread, forcing the project system to synchronize initialize it (6.9% of solution open) #55297

Comments

davkean commented Aug 11, 2016 • edited Loading

davkean commented Sep 15, 2016

mavasani commented Dec 14, 2016

Pilchie commented Dec 19, 2017

Pilchie commented Jan 18, 2018

davkean commented Nov 7, 2018

davkean commented Nov 7, 2018

davkean commented Nov 7, 2018 • edited Loading

drewnoakes commented Feb 1, 2021 • edited Loading

tmeschter commented Feb 2, 2021

davkean commented Feb 2, 2021 via email • edited by ghost Loading

drewnoakes commented Feb 5, 2021

drewnoakes commented Feb 10, 2021

drewnoakes commented Feb 12, 2021

CyrusNajmabadi commented Feb 19, 2021

drewnoakes commented Feb 19, 2021

drewnoakes commented Feb 19, 2021

This comment has been minimized.

drewnoakes commented May 12, 2021

CyrusNajmabadi commented Jul 30, 2021

davkean commented Jul 30, 2021

jasonmalinowski commented Sep 13, 2021

davkean commented Sep 13, 2021 • edited Loading

CyrusNajmabadi commented Sep 14, 2021

jasonmalinowski commented Mar 22, 2022

davkean commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

davkean commented Mar 22, 2022 • edited Loading

jasonmalinowski commented Mar 22, 2022

jasonmalinowski commented Mar 22, 2022

davkean commented Aug 11, 2016 •

edited

Loading

davkean commented Nov 7, 2018 •

edited

Loading

drewnoakes commented Feb 1, 2021 •

edited

Loading

davkean commented Feb 2, 2021 via email •

edited by ghost

Loading

davkean commented Sep 13, 2021 •

edited

Loading

davkean commented Mar 22, 2022 •

edited

Loading