perforce / P4VS

[Officially Supported] VisualStudio plugin for HelixCore
Other
15 stars 6 forks source link

Extension causing responsiveness issues inside Visual Studio #11

Closed davkean closed 4 months ago

davkean commented 1 year ago

Hey folks, I'm from the Visual Studio Performance and Reliability team and our telemetry has captured some performance problems with this extension in the wild.

It appears that under certain circumstances, the following stack performs an extraordinarly large amount of allocations, bringing Visual Studio to a crawl while the GC tries to clean up these allocations.

We've caught the issue 3 times in the past 21 days, and at its worst is allocating over 1 GB/second.

Name
         + mscorlib.ni!String.Concat
          + p4api.net!Perforce.P4.P4Exception..ctor(class System.String,class System.String[],class Perforce.P4.P4ClientErrorList,class Perforce.P4.P4ClientInfoMessageList)
           + p4api.net!P4Exception.Throw
            + p4api.net!P4Server.RunCommand
             + p4api.net!P4Command.RunInt
              + p4api.net!Perforce.P4.P4CommandResult..ctor(class Perforce.P4.P4Command,class Perforce.P4.StringList)
               + p4api.net!P4Command.Run
                + p4api.net!Repository.GetFileMetaData
                 + p4vs_utils!P4ScmCache.UpdateFiles
                  + p4vs_utils!P4ScmCache.RefreshFilesThreadProc

The issue appears to stem from this line in the exception's constructor where it looks like what is already likely a very large command-line is being concatenated to a large number of arguments. Each one of the new strings being created are larger than 85KB as I can see they are all being pushed on the Large Object Heap. In one 60 second trace we automatically captured, this one loop produced 68 GB of allocations.

Please let me know if you need some help understanding/fixing this. We don't have a repro for this, as it was automatically captured by our telemetry system.

-Dave

skardile-perforce commented 1 year ago

Thanks @davkean for this insight.

davkean commented 1 year ago

This will only occur when the constructor for P4Exception is called, which I assume is only created when that exception is being thrown.

Unfortunately, this has been caught by our performance telemetry on real customer machines and we don't capture enough information to see the contents of the exception, nor can we turn on logging on those machines. By looking at the code you can see that a very large command line with a large number of arguments will result in this problem.

davkean commented 1 year ago

I should add, all of these strings are ending up on the Large Object Heap (LOH), which requires an object allocation of 85 KB, which takes a string of 42,500 characters.

skardile-perforce commented 1 year ago

Okay. I will look for such a combination where this large command is formed.

oold commented 5 months ago

Anything being done about this issue? Seems like a matter of simply replacing the line concatenating the command arguments one by one with a StringBuilder. Could even pre-allocate the buffer since all the strings to be appended are already part of a collection.

shraddhaborate commented 4 months ago

This issue has been fixed for P4API.NET and will be available in next P4VS release.

davkean commented 3 months ago

Thanks @shraddhaborate and folks.