Fix race with boosting OOP priority. Also, ensure FindRef calls are boosted. #20174
Conversation
|
Tagging @dotnet/roslyn-ide @heejaechang @Pilchie easier to review as: https://github.com/dotnet/roslyn/pull/20174/files?w=1 |
|
Previous code would manipulate a value, then set state according to that value. However, nothing was ensuring that this happened atomically. Making it possible for code to need to be boosted, but end up unboosted. Unboosted code that is invoked by a user is not good. It may take a long itme to run if VS is using the available CPU cycles, and the OOP process gets almost nothing due to it running below-normal pri by default. |
|
@CyrusNajmabadi thank you for fix this. my bad! |
|
|
||
| private static void Start() | ||
| { | ||
| var value = Interlocked.Increment(ref s_count); |
There was a problem hiding this comment.
I don't see why this is actually a bug.
There was a problem hiding this comment.
count is fine. changing priority can be reordered and run in reverse order.
There was a problem hiding this comment.
For example, we could have the following:
BG code is running (refcount is 1).
BG code ends, drops ref count to 0.
User code runs, increases refcount to 1.
User code sets priority to normal
BG code now runs and sets priority to low.
|
Tagging @MattGertz Customer scenario Some user-invoked OOP operations make take much longer if VS is using a lot of CPU. Because VS and OOP have to share the same CPU resources and OOP runs 'below normal priority' by default, then work in the OOP process may starve while VS is consuming lots of CPU. As these are user-invoked features, that's a very detrimental user experience as tehy may be waiting a long time for their results to appear. Bugs this fixes: N/A. Workarounds, if any None. Risk Low. Performance impact user observable performance improves. Is this a regression from a previous update? Yes (if you turn on OOP). Root cause analysis: The OOP system runs low-priority by default to keep the OOP server from negatively impacting VS while it's just doing background work. However, if OOP is doing work explicitly requested by the user (like Navigate-To/FindRefs/etc.) then we 'boost' the priority to 'normal' so that the OOP server gets a reasonable amount of CPU time. A race condition in this 'boost' code made it so that the OOP code might not actually happen, leading to OOP running low priority even when working to do a user request. How was the bug found? Dogfooding. |
No description provided.