Add Dump command to VS #4580

AviAvni · 2018-03-19T20:13:27Z

Talking with @dsyme I implemented Dump command to vs to help anlyze memory issues of F# language service in VS
The dump taken after all operations in Reactor finished
And I used ClrMD microsoft/clrmd#91 to take the dump so it not waste disk or ram
The dump written to the output window
@cartermp what do you think?
I'm also want to implement this on tests so we can compare memory after every build

One thing missing is thaat even that I added the Microsoft.Diagnostics.Runtime package to FSharp.Editor
and VisualFSharpFull projects the dll is not included in the vsix how I can fix this?

cartermp

Giving this "experience approval", in that I think this is a valuable tool and that it's worth having.

@Pilchie any thoughts on introducing another menu item here?

majocha · 2018-03-20T18:24:38Z

@AviAvni adding to VisualFSharpFull.csproj

  <ItemGroup>
      <VSIXSourceItem Include="$(OutputPath)Microsoft.Diagnostics.Runtime.dll" />
  </ItemGroup>

should probably work, but I have no idea if this is the recommended way to do it.

brettfo · 2018-03-21T20:08:33Z

vsintegration/src/FSharp.ProjectSystem.FSharp/MenusAndCommands.vsct

        </Strings>
      </Button>
+
+      <Button guid="FSharpProjectCmdSet" id="cmdidFsDump" priority="0x8000" type="Button">


This addition should have (but obviously didn't) update all of the MenusAndCommands.vsct.*.xlf files. Can you try running this to force regeneration?

msbuild vsintegration\src\FSharp.ProjectSystem.FSharp\ProjectSystem.fsproj /t:UpxateXlf

brettfo · 2018-03-21T20:10:38Z

vsintegration/Vsix/VisualFSharpFull/VisualFSharpFull.csproj


  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonPackageVersion)" />
+    <PackageReference Include="Microsoft.Diagnostics.Runtime" Version="0.9.180305.1" />


This version number should be added to build\targets\PackageVersions.props under the other packages heading and placed in alphabetical order:

<MicrosoftDiagnosticsRuntimePackageVersion>0.9.180305.1</MicrosoftDiagnosticsRuntimePackageVersion>

And replaced in this file with $(MicrosoftDiagnosticsRuntimePackageVersion).

brettfo · 2018-03-21T20:12:35Z

vsintegration/src/FSharp.Editor/Commands/DumpCommandService.fs

+    let projectSystemPackage =
+      lazy(
+        let shell = serviceProvider.GetService(typeof<SVsShell>) :?> IVsShell
+        let packageToBeLoadedGuid = ref (Guid "{91a04a73-4f2c-4e7c-ad38-c1a68e7da05c}")  // FSharp ProjectSystem guid        


Use the constant defined ProjectPrelude.fs as GuidList.guidFSharpProjectPkgString.

brettfo · 2018-03-21T20:13:40Z

vsintegration/src/FSharp.Editor/Commands/DumpCommandService.fs

+        checkerProvider.Checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()
+        use target = DataTarget.CreateSnapshotAndAttach(Process.GetCurrentProcess().Id)
+        let runtime = target.ClrVersions.[0].CreateRuntime();
+        let outputPane = projectSystemPackage.Value.GetOutputPane(VSConstants.OutputWindowPaneGuid.BuildOutputPane_guid, "FSharp Dump")


I'm not familiar with this API. Does "FSharp Dump" need to be localized/translated? If so it'll have to be added to a .resx file and consumed that way.

brettfo · 2018-03-21T20:14:09Z

vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj

    <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonPackageVersion)" />
    <PackageReference Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutablePackageVersion)" />
    <PackageReference Include="VSSDK.VSLangProj" Version="$(VSSDKVSLangProjPackageVersion)" />
+    <PackageReference Include="Microsoft.Diagnostics.Runtime" Version="0.9.180305.1" />


Same build\targets\PackageVersions.props comment as above.

Pilchie · 2018-03-21T20:40:48Z

Sorry for the late reply - was on vacation. I would tend to prefer that this be added as an extension, perhaps into the "Project System Tools" extension? Tagging @panopticoncentral

AviAvni · 2018-03-21T20:56:23Z

@brettfo and @Pilchie thanks for feedback
This is in the "Project System Tools" because @dsyme wanted it in a specific state of the language service when all the work on the queue is done
In any case I'll be happy to continue work on this

panopticoncentral · 2018-03-22T17:37:44Z

This seems like it could be a good addition to the project system tools (https://github.com/dotnet/project-system-tools) and it fits with the general mission there. We've already added logging for the Roslyn language service. Take a look and let me know what you think, @AviAvni.

AviAvni · 2018-03-22T17:51:19Z

@panopticoncentral ok so how this is going to work?
can I use classes that declared in this repo there via nuget so I can take the dump when the lang service finished all operations
or I'll just do the dump there and it's need to be taken manually on time
or I'll expose it as a service and use that service here
or you have another idea?

panopticoncentral · 2018-03-30T20:38:47Z

Is the API to know when the lang service is done available publicly? If so, I'd just call it from the extension and then do the dump.

KevinRansom · 2018-10-02T14:56:16Z

@AviAvni

Hello my friend, is this still an active work item for you?

Thanks
Kevin

AviAvni · 2018-10-02T15:19:38Z

I can work on it if you want but as said in comments it belong to other project that I don't know how to implement it there

cartermp · 2018-10-02T15:26:27Z

Since @panopticoncentral is/will be on leave (I've been out myself and forgot the status!), @jmarolf would you be able to help @AviAvni with adding a Dump command to https://github.com/dotnet/project-system-tools ?

KevinRansom · 2019-03-09T23:01:01Z

Thanks for this PR, however it looks to be quite out of date, please reopen, if you want to continue with it.

Kevin

AviAvni added 9 commits March 14, 2018 20:46

add dump command

d5a6a68

Merge branch 'master' into memory-analysis

0e9ef10

fix debugger

b463580

Merge branch 'master' into memory-analysis

d732bdf

implement dump command

0b2f885

fix dump result

0e7f77c

add new line

d2bfcfe

fix guid and spelling

f8b75f0

fix warnings and change guid

b35ee40

cartermp approved these changes Mar 20, 2018

View reviewed changes

AviAvni mentioned this pull request Mar 21, 2018

Use Roslyn for backing metadata bytes in VS #4586

Merged

4 tasks

brettfo suggested changes Mar 21, 2018

View reviewed changes

KevinRansom closed this Mar 9, 2019

Add Dump command to VS #4580

Add Dump command to VS #4580

Uh oh!

Conversation

AviAvni commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cartermp left a comment

Choose a reason for hiding this comment

Uh oh!

majocha commented Mar 20, 2018

Uh oh!

brettfo Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

brettfo Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

brettfo Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

brettfo Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

brettfo Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pilchie commented Mar 21, 2018

Uh oh!

AviAvni commented Mar 21, 2018

Uh oh!

panopticoncentral commented Mar 22, 2018

Uh oh!

AviAvni commented Mar 22, 2018

Uh oh!

panopticoncentral commented Mar 30, 2018

Uh oh!

KevinRansom commented Oct 2, 2018

Uh oh!

AviAvni commented Oct 2, 2018

Uh oh!

cartermp commented Oct 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KevinRansom commented Mar 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AviAvni commented Mar 19, 2018 •

edited

Loading

brettfo Mar 21, 2018 •

edited

Loading

cartermp commented Oct 2, 2018 •

edited

Loading