search

dotnet / BenchmarkDotNet

Powerful .NET library for benchmarking
https://benchmarkdotnet.org
MIT License
10.47k stars 962 forks source link

Simplify dotTrace diagnoser, dedupe common code #2558

Closed martinothamar closed 1 month ago

martinothamar commented 5 months ago

Similar approach to #2549 for simplifying DotTraceDiagnoser.

Also deduped some code by creating a shared project (using <Compile Include="..." /> to avoid another package). Pulled this code into JetBrains folder so that the projects are ordered next to eachother in file explorers. Made sure to update build script and references to include the new folder.

Other options:

timcassell commented 5 months ago

Could you use shared project for this?

image

martinothamar commented 5 months ago

TIL, did not know that existed :smile:

I wanted to test that out now but I'm not sure if it's possible to create that project type for me since I'm working on Linux atm. .NET SDK, Rider and VSCode C# DevKit all seem to be missing support for creating this project type.

Reading up on this issue: https://github.com/dotnet/sdk/issues/2511 There seems to be some complaints about the UX of it, but that it is fully usable. Some people suggest Microsoft.Build.NoTargets, while the "official" recommendation seems to be using a folder + globbing includes? I thought having a plain folder to contain the files would make intellisense/tooling/IDE experience worse but testing that out now its completely fine and everything works as expected. So personally I think the plain folder approach seems to be nice enough and lightweight. I pushed a commit now to do that

I could try out shared projects, but I would have to hand-code it unless someone on a Windows PC could scaffold it for me. What do you think?

timcassell commented 5 months ago

Ah, I didn't realize that was legacy project type. I think just using the folder approach is good.

AndreyAkinshin commented 1 month ago

@martinothamar thanks for the PR! Indeed, it makes sense to unify dotTrace/dotMemory diagnosers and eliminate code duplication. However, the presented approach inherits the disadvantages of the poor design of the original diagnosers. Both were born as prototypes. Now, it's time to refactor them. I have come up with a better solution for this problem, see #2627.