Allow for better parallelization with brokered services - #82733
Conversation
By default, there is a SynchronziationContext set that ensures that when dispatching new RPC requests, a new RPC requests is not dispatched until the previous request either finishes, or awaits to yield the thread. Although there are some cases where we do need behavior like that (LSP is the perfect example), we don't need that for brokered services.
| // The default synchronization context set by the base type is NonConcurrentSynchronizationContext, which means that all incoming calls | ||
| // would be processed sequentially, at least up until their first await. We generally don't have services that expect ordering in that specific way, so | ||
| // disable that, especially since we want CPU-bound operations to happen in parallel. | ||
| jsonRpc.SynchronizationContext = null; |
There was a problem hiding this comment.
Does this apply to all brokered service usages - or just devenv -> oop? (e.g. does this apply to vscode brokered services)
There was a problem hiding this comment.
@dibarbet As best I know, everything, VS Code included.
|
View PR Validation Run triggered by @jasonmalinowski Parameters
|
|
The internal validation build is showing allocation regressions in RemoteHostAssetWriter.WriteBatchToPipeAsync. I have two theories for this:
I've kicked off an internal build to test the second theory by just adding a bit wait at the end of the test. |
|
With @kayle's help I got a run with an extra wait at the end. Although I see more work being done, I don't see more work through the RemoteHostAssetWriter, which points towards option 1 being the problem here. |
|
@jasonmalinowski do you have any theories about this? |
|
@drewnoakes Nothing better than what I've posted. My attention has been elsewhere for the last few days. |
|
View PR Validation Run triggered by @jasonmalinowski Parameters
|
|
Re-ran a validation run internally, and it showed no regressions. Going to run a second one just to be sure. |
|
And the second run also shows no regressions. So @drewnoakes and @dibarbet unless you have any other concerns, will go ahead and merge this. Your reviews are quite old but no actual change to the code. |
) Reverts #82733. This still [seemed to cause perf regressions](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/3044775), even though the runs from the PR validation were fine.
5 participants
By default, there is a SynchronizationContext set that ensures that when dispatching new RPC requests, a new RPC requests is not dispatched until the previous request either finishes, or awaits to yield the thread. Although there are some cases where we do need behavior like that (LSP is the perfect example), we don't need that for brokered services.