The Doki-Theme causing Visual Studio unresponsiveness

[davkean]

Hey folks, I'm from the Visual Studio team and our performance telemetry has caught an issue in this extension where it is causing Visual Studio unresponsivess by consuming large number of thread pool threads in the wild.

Over the past 90 days, we've captured 14 instances of this, with an average of 37 threads, but upwards of 269 threads.

Failure	Cab Count	Threads 50th	Threads 90th	Avg Threads
doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.ResolveAssetUrl.AnonymousMethod__0	14	17	60	37

In one of the dumps we captured, I see that the following stack is consuming 269 threads trying to retrieve UserLocalDataPath.

    ntdll.dll!ZwWaitForMultipleObjects() Line 907   Unknown
    KERNELBASE.dll!WaitForMultipleObjectsEx(unsigned long nCount, void * const * lpHandles, int bWaitAll, unsigned long dwMilliseconds, int bAlertable) Line 1551   C
    [Managed to Native Transition]  
    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<System.__Canon>.GetResultCore(bool waitCompletionNotification) Unknown
    [Frames below may be incorrect and/or missing, no binaries loaded for Microsoft.VisualStudio.Shell.UI.Internal.dll] 
    [Native to Managed Transition]  
    msenv.dll!CThreadAwareServiceProvider::QueryService(__VSTASKRUNCONTEXT taskPriority, const _GUID & guidService, const _GUID & riid, void * * ppv) Line 553  C++
    msenv.dll!CThreadAwareServiceProvider::QueryService(const _GUID & guidService, const _GUID & riid, void * * ppv) Line 342   C++
    [Managed to Native Transition]  
    Microsoft.VisualStudio.Shell.Framework.dll!Microsoft.VisualStudio.Shell.ServiceProvider.QueryService(System.Guid guid, System.Type serviceType, bool setShellErrorInfo, out object service) Line 238    C#
    Microsoft.VisualStudio.Shell.Framework.dll!Microsoft.VisualStudio.Shell.ServiceProvider.QueryService(System.Type serviceType, bool setShellErrorInfo, out object service) Line 144  C#
    Microsoft.VisualStudio.Shell.Framework.dll!Microsoft.VisualStudio.Shell.ServiceProvider.GetService(System.Type serviceType, bool setShellErrorInfo) Line 110    C#
    Microsoft.VisualStudio.Shell.15.0.dll!Microsoft.VisualStudio.Shell.Package.GetService(System.Type serviceType) Line 1046    C#
    Microsoft.VisualStudio.Shell.15.0.dll!Microsoft.VisualStudio.Shell.AsyncPackage.GetService(System.Type serviceType) Line 1196   C#
    Microsoft.VisualStudio.Shell.15.0.dll!Microsoft.VisualStudio.Shell.Package.GetRegistryRoot() Line 926   C#
    Microsoft.VisualStudio.Shell.15.0.dll!Microsoft.VisualStudio.Shell.Package.UserLocalDataPath.get() Line 172 C#
    doki-theme-visualstudio.dll!doki_theme_visualstudio.LocalStorageService.GetAssetDirectory() Unknown
    doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.ConstructLocalAssetPath(doki_theme_visualstudio.AssetCategory assetCategory, string assetPath) Unknown
    doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.ResolveAssetAsync(doki_theme_visualstudio.AssetCategory assetCategory, string assetPath, string assetSource, System.Func<string, string, System.Threading.Tasks.Task<string>> resolveAsset)    Unknown
    doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.CachedResolveAsync(doki_theme_visualstudio.AssetCategory assetCategory, string assetPath, string assetSource)  Unknown
    doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.ResolveAssetUrlAsync(doki_theme_visualstudio.AssetCategory assetCategory, string assetPath)    Unknown
    doki-theme-visualstudio.dll!doki_theme_visualstudio.AssetManager.ResolveAssetUrl.AnonymousMethod__0()   Unknown

To resolve this, please capture UserLocalDataPath once on the UI thread in https://github.com/doki-theme/doki-theme-visualstudio/blob/main/doki-theme-visualstudio/doki_theme_visualstudioPackage.cs#L54, and then pass that through to the components that need it.

[Unthrottled]

Are you sure that is the issue? (not a visual studio/dot net wiz)

Is calling UserLocalDataPath on the AsycPackage while not on the UI thread the actual issue? So just capturing UserLocalDataPath from the package while in the InitializeAsync will fix it? I suppose this is a dead lock kind of scenario? (tried reading the stack trace, assumed lower is higher up in the call chain, and the highest is the active method stuck)

https://github.com/doki-theme/doki-theme-visualstudio/blob/872a1cc45d9e8f18545de88afd8641e8e6722695/doki-theme-visualstudio/LocalStorageService.cs#L34

I would have guessed that the issue might be slow network speeds, because the extension downloads assets from the web to be used by the extension. Which is why I was sure to not run that download work on the UI thread. So that might be causing the issue because it tries to get the path while outside the UI thread? Perhaps it might be because it takes forever for it to download or it could be like you said and is an issue with deadlock or something.

Any insights are welcome.

@davkean

[davkean]

The problem is that UserLocalDataPath under the covers performs a hidden thread switch to the UI thread, in times of high UI thread usage (such as solution load), these calls can back up waiting for access, consuming large amounts of thread pool threads in the meantime. This prevents other features from using these threads.

In the above dump, we had 269 threads (ie calls) backed up all waiting for access to the UI thread. A typical VS instance should have only as many thread pool threads as there are logical CPUs (4 on a 4-core machine, 8 on a 4-core with hyper-threading for example), but this extension is consuming 269 of them (!), at which point VS would have slowed to a crawl and would very unpleasant to use.

By caching and reading UserLocalDataPath once upfront, this thread switch is avoided and removes the backup of threads.

Towards the top of the stack, you can see a call to CThreadAwareServiceProvider::QueryService, that is performing the thread switch to grab a "service" from the UI thread, so its not network related.

[Unthrottled]

Thanks for bringing this to my attention. Pretty sure I addressed the issue. Ran the code that accesses UserLocalDataPath on the UI thread, captured it for reuse later on.

[davkean]

Thanks, @Unthrottled looks good.