VisualStudioWorkspace.WorkspaceChanged should be replaced with an off-UI-thread solution #78091
Description
Workspaces have a WorkspaceChanged event which is, in Visual Studio for VisualStudioWorkspace, is raised on the UI thread. Historically, updates to the solution happened on the UI thread anyways, but as more and more of our project system interactions have moved off the UI thread, this is still a remnant of it. However, a lot of uses of the event don't need the UI thread, and could easily be moved to a background thread.
The "simplest" change might be to just move where the event is invoked, but that would be a pretty breaking change for the few things that do care. So maybe we should deprecate WorkspaceChanged (or at least deprecate it for in-proc Visual Studio components),and offer a more modern API here. I'm thinking something like this:
public class Workspace
{
public WorkspaceChangedRegistration RegisterWorkspaceChangedHandler(Func<WorkspaceChangedEventArgs, Task> handler, WorkspaceChangedEventOptions options) { ... }
}
public class WorkspaceChangedRegistration : IDisposable { }The shape away from a traditional event handler is intentional. First, this makes it easy for us to take async handlers, so we can invoke on a background thread and if a handler must move to the UI thread, they can do so. We could, under the covers, consider each registration to be a separate 'queue' each which get notified at their own speeds. So, one feature is doing some async they're not holding up other processing. Or we could still consider it one queue.
Second, it also means we can take an optional set of options which control how often we send notifications. For example, taggers subscribe to workspace changed to see if something they care about (parse options, etc.) that impact the current file. Right now they're seeing all changes in a stream but we could:
- Just filter out events that only impact the project/document they're watching, to avoid queueing work just to filter it later.
- Give an option for a subscriber to say "I only need a summarized workspace changed every half second or so", which then means we don't even have to queue up all the work but just can create a single event with an old/new and be done with it.
For the benefit of 2, it may not be terribly important for internal-to-Roslyn consumers since we tend to often feed the work into an AsyncBatchingWorkQueue, but for code that isn't using that (or can't because it's in other parts of VS) having something that is ready-made might be helpful. And it might be trivial for us to implement atop AsyncBatchingWorkQueue anyways. But thta can be treated as optional if we want to do a first pass and just get most folks off the UI thread which would be an easy win.
The return type is IDisposable so you can dispose it to signal you're done with your event subscription. I figure rather than just having the method return IDisposable, returning a concrete type allows for a bit more flexibility down the road. For example, right now a consumer has no way to say "wait until I've seen all the notifications up to this point" which we could then offer. When I did a cursory scan of uses in Roslyn I noticed one or two places where that might be better than what we might be doing now.
If we provided something like this and at least got enough moved off of WorkspaceChanged such that it had no subscribers in a base install of VS, it then means we could avoid raising the event and jumping to the UI thread at all.
(This is coming out of a conversation I had with @ToddGrun as we're thinking of other ways to improve solution load performance. Before we do work here we should measure to make sure the cost of what we're doing on the UI thread is worth the engineering effort to fix.)