madskristensen / RainbowBraces

A Visual Studio extension
Apache License 2.0
132 stars 7 forks source link

High CPU usage #61

Closed madskristensen closed 1 year ago

madskristensen commented 1 year ago

We have a callstack showing some threads going crazy in the extension:


Name
--
\|\|  + mscorlib.ni!ThreadPoolWorkQueue.Dispatch$##6003CA8
\|\|   + mscorlib.ni!IThreadPoolWorkItem.ExecuteWorkItem$##6003CC9
\|\|    + mscorlib.ni!System.Runtime.CompilerServices.AsyncMethodBuilderCore+MoveNextRunner.Run()$##6006FC7
\|\|     + mscorlib.ni!ExecutionContext.Run$##6003AEF
\|\|      + mscorlib.ni!ExecutionContext.RunInternal$##6003AF0
\|\|       + RainbowBraces!RainbowBraces.RainbowTagger+d__25.MoveNext()
\|\|       \|+ RainbowBraces!BracePairBuilderCollection.SaveToCache
\|\|       \| + mscorlib!System.Collections.Generic.Dictionary`2[System.ValueTuple`2[System.Char,System.Char],System.__Canon].Insert(!0,!1,bool)

It looks like we are saving to cache too frequently.

@lordfanger what do you think?

lordfanger commented 1 year ago

Looks more like concurrency issue. Default dictionary capacity is 3. I've added fourth pair ('<' and '>') and then the dictionary can be in corrupted state and can end in infinite loop.

The SaveToCache() should be called once per ParseAsync() and set up to 4 entries.

I'll have a look.

davkean commented 1 year ago

Our Visual Studio telemetry that monitors high-CPU also captured this problem, thanks for the fix @lordfanger!

lordfanger commented 1 year ago

I'm sorry I read the callstack wrong. The problem was in async await method (RainbowBraces!RainbowBraces.RainbowTagger+d__25.MoveNext()). And somehow I overlooked it previously. It should be fixed now in #66.