Skip to content

Conversation

martinothamar
Copy link
Contributor

Implements DotMemoryDiagnoser to match DotTraceDiagnoser

  • The code is pretty similar between dotMemory and dotTrace impl, because the underlying APIs/CLIs are similar. Could do something to share code here
    • Is there any reason we can't use the PID config on the APIs for the Jetbrains library directly? I could refactor to use that instead, both for dotTrace and dotMemory
  • Could refactor out common code and either publish a Common package or just use <Compile Include="..." />

Sample output of IntroDotMemoryDiagnoser

image

@martinothamar
Copy link
Contributor Author

There was a segfault happening for the integration test that I don't quite understand. I can no longer reproduce it after the last commit. I tried to debug some dumps locally but was unable to find relevant exceptions, so my only guess atm is that the crash originates from unmanaged code. Since changing the init logic seemed to affect things, and the crash always happening during InProcessEmitToolchain, I'm guessing it has something to do with attaching/detaching multiple times in the same process or something

In any case, I would like to test out using only the JetBrains API directly

@martinothamar
Copy link
Contributor Author

martinothamar commented Mar 31, 2024

The last commit simplifies the implementation of the tool by using only the Jetbrains API, and the lifetime of it is more clearly managed based on the incoming signals.

I'd be willing to try the same with dotTrace impl. I also want to make a flag like performExtraBenchmarksRun similar to some of the other profilers. I could do these en subsequent PRs

@AndreyAkinshin AndreyAkinshin merged commit c7b7abf into dotnet:master Apr 1, 2024
@AndreyAkinshin AndreyAkinshin added this to the v0.14.0 milestone Apr 1, 2024
@AndreyAkinshin
Copy link
Member

@martinothamar thank you very much for this PR!

@martinothamar martinothamar deleted the feature/dotMemory-profiler branch April 1, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants