The EventPipeProfiler cross-platform profiler #1321

WojciechNagorski · 2019-11-27T13:10:06Z

Changes for #1315

Currently, I use a local build of Microsoft.Diagnostics.NETCore.Client from dotnet/diagnostics#617. This library support only .Net Core 2.1 so I decided that I create a new library BenchmarkDotNet.Diagnostics.NETCore that contains EventPipeProfiler.

(I don't like the name "NETCore" but I wanted to do it like in Microsoft.Diagnostics.NETCore.Client )

You can run this profiler via command line:

dotnet BenchmarkDotNet.Samples.dll --filter *Algo_Md5VsSha256.Md5* -p EventPipe -j Short

Example trace file.zip

…to task-EventPipeProfiler

adamsitnik · 2019-12-16T13:59:12Z

I've tested the proposed EventPipeProfiler and it works fine. Great work @WojciechNagorski !

I've sent dotnet/diagnostics#700 in order to add .NET Standard 2.0 support to Microsoft.Diagnostics.NETCore.Client so we could just add this diagnoser to existing package without adding .NET Core target.

WojciechNagorski · 2019-12-16T14:36:51Z

@adamsitnik Thanks. I'm going to finish this task this week.

adamsitnik · 2019-12-17T09:19:57Z

dotnet/diagnostics#700 got merged and it targets .NET Standard 2.0 now, so IMHO we can add this dependency to main BenchmarkDotNet project and just add the new profiler plugin there

WojciechNagorski · 2019-12-17T09:22:58Z

@adamsitnik Great. Today I will move EventPipeProfiler to the BenchmarkDotNet project.

…to task-EventPipeProfiler

WojciechNagorski · 2019-12-18T15:00:38Z

I've pushed the updated version of EventPipeProfiler. It is still WIP, I need to test all cases.

adamsitnik

@WojciechNagorski thanks for another great PR!

Please address my comments before we merge it. Thanks!

adamsitnik · 2019-12-18T15:32:09Z

BenchmarkDotNet.sln

-# Visual Studio 15
-VisualStudioVersion = 15.0.27130.2027
+# Visual Studio Version 16
+VisualStudioVersion = 16.0.29102.190


please revert all the changes to solution file

I will revert the version of Visual Studio. But I think that guilds should be changed, dotnet/project-system#3079 (comment)
and
dotnet/project-system#2863
What do you think?

src/BenchmarkDotNet/Diagnosers/DiagnosersLoader.cs

src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs

src/BenchmarkDotNet/Toolchains/ToolchainExtensions.cs

src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs

src/BenchmarkDotNet/Diagnosers/ListProfilesCommandHandler.cs

WojciechNagorski · 2019-12-18T22:02:51Z

@adamsitnik I made the changes you asked for. I have questions for two topics.

…to task-EventPipeProfiler

WojciechNagorski · 2020-03-02T12:02:42Z

@adamsitnik ping

adamsitnik

Looks great to me, thank you @WojciechNagorski !

Please let me know if you have some time to address my feedback today. If not, I am going to merge it as it is and send a minor improvement PR.

Once again big thanks!

src/BenchmarkDotNet/BenchmarkDotNet.csproj

src/BenchmarkDotNet/Diagnosers/EventPipeProfileMapper.cs

adamsitnik · 2020-03-03T08:58:34Z

src/BenchmarkDotNet/Diagnosers/EventPipeProfile.cs

+    {
+        CpuSampling,
+        GcVerbose,
+        GcCollect


it would be great to add Jit to this list with inlining and tail call info enabled

src/BenchmarkDotNet/Diagnosers/EventPipeProfileMapper.cs

adamsitnik · 2020-03-03T09:04:22Z

src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs

+            Path.GetDirectoryName(fileName).CreateIfNotExists();
+            benchmarkToTraceFile[parameters.BenchmarkCase] = fileName;
+
+            collectingTask = new Task(() =>


nit: we have quite a deep nesting here. in a perfect world, we would move this logic to a separate method and avoid closure capture

adamsitnik · 2020-03-03T09:06:16Z

src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs

+
+        public string ShortName => "EventPipe";
+
+        private void ConvertToSpeedscope(BenchmarkCase benchmarkCase, string traceFilePath)


this class is responsible for profiling using event pipe (EventPipeProfiler) could you please move everything that is related to exporting to speedscope to a dedicated type?

adamsitnik · 2020-03-03T09:06:28Z

src/BenchmarkDotNet/Diagnosers/EventPipeProfiler.cs

+            }
+        }
+
+        // Method copied from https://github.com/dotnet/diagnostics/blob/2c23d3265dd8f642a8d6cf4bb8a135a5ff8b00c2/src/Tools/dotnet-trace/TraceFileFormatConverter.cs#L64


thanks for adding the comment!

adamsitnik · 2020-03-03T09:07:37Z

src/BenchmarkDotNet/Engines/EngineEventSource.cs

    public class EngineEventSource : EventSource
    {
+        const string SourceName = "BenchmarkDotNet.EngineEventSource";


it looks like there is no need to extract const if it's used only in once place. Could you please revert this change?

src/BenchmarkDotNet/Helpers/ArtifactFileNameHelper.cs

adamsitnik

It looks like I need to approve it again after pushing GH suggestions...

WojciechNagorski · 2020-03-03T11:01:54Z

@adamsitnik I will try to fix it now.

adamsitnik · 2020-03-03T11:19:11Z

I will try to fix it now.

@WojciechNagorski don't worry about this, I am going to send a small PR with the improvements

WojciechNagorski · 2020-03-03T11:31:20Z

@adamsitnik Ok, so I won't change anything. There you can find what I've done so far: #1382

WojciechNagorski added 2 commits November 27, 2019 13:32

First version of EventPipeProfiler

e92297a

Merge branch 'master' of https://github.com/dotnet/BenchmarkDotNet in…

1f5dd79

…to task-EventPipeProfiler

WojciechNagorski mentioned this pull request Dec 10, 2019

Implement Unix Disassembler for .NET Core #1316

Closed

adamsitnik mentioned this pull request Dec 16, 2019

[Proposal] add .NET Standard 2.0 for diagnostics client library dotnet/diagnostics#700

Merged

WojciechNagorski added 5 commits December 18, 2019 09:58

Merge branch 'master' of https://github.com/dotnet/BenchmarkDotNet in…

cd511cc

…to task-EventPipeProfiler

Improve EventPipeProfiler

0ffa20d

Improve EventPipeProfiler

c1ea50c

Fix BenchmarkDotNet.EngineEventSource

1fa2df3

Fix build

06cd124

adamsitnik reviewed Dec 18, 2019

View reviewed changes

Fix review

8cef61a

WojciechNagorski changed the title ~~[WIP] EventPipeProfiler~~ The EventPipeProfiler cross-platform profiler Dec 18, 2019

Small refactor

c78b6a5

WojciechNagorski mentioned this pull request Dec 19, 2019

Attribute improvements #1335

Merged

WojciechNagorski requested a review from adamsitnik January 10, 2020 05:30

Merge branch 'master' of https://github.com/dotnet/BenchmarkDotNet in…

9985c39

…to task-EventPipeProfiler

WojciechNagorski mentioned this pull request Jan 13, 2020

Implement cross platform EventPipeProfiler diagnoser #1315

Closed

WojciechNagorski added 3 commits March 2, 2020 11:21

Merge branch 'master' of https://github.com/dotnet/BenchmarkDotNet in…

c9ceb95

…to task-EventPipeProfiler

Merge branch 'master' of https://github.com/dotnet/BenchmarkDotNet in…

dee6839

…to task-EventPipeProfiler

Update TraceEvent from PR dotnet#1376

97bf6e9

adamsitnik previously approved these changes Mar 3, 2020

View reviewed changes

Apply suggestions from code review

2319cf7

adamsitnik dismissed their stale review via 2319cf7 March 3, 2020 09:40

adamsitnik approved these changes Mar 3, 2020

View reviewed changes

adamsitnik merged commit c648ff9 into dotnet:master Mar 3, 2020

AndreyAkinshin added this to the v0.12.1 milestone Mar 3, 2020

WojciechNagorski deleted the task-EventPipeProfiler branch March 3, 2020 11:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The EventPipeProfiler cross-platform profiler #1321

The EventPipeProfiler cross-platform profiler #1321

WojciechNagorski commented Nov 27, 2019 •

edited

Loading

adamsitnik commented Dec 16, 2019

WojciechNagorski commented Dec 16, 2019

adamsitnik commented Dec 17, 2019

WojciechNagorski commented Dec 17, 2019

WojciechNagorski commented Dec 18, 2019

adamsitnik left a comment

adamsitnik Dec 18, 2019

WojciechNagorski Dec 18, 2019

WojciechNagorski commented Dec 18, 2019

WojciechNagorski commented Mar 2, 2020

adamsitnik left a comment

adamsitnik Mar 3, 2020

adamsitnik Mar 3, 2020

adamsitnik Mar 3, 2020

adamsitnik Mar 3, 2020

adamsitnik Mar 3, 2020

adamsitnik left a comment

WojciechNagorski commented Mar 3, 2020

adamsitnik commented Mar 3, 2020

WojciechNagorski commented Mar 3, 2020


		public string ShortName => "EventPipe";

		private void ConvertToSpeedscope(BenchmarkCase benchmarkCase, string traceFilePath)

The EventPipeProfiler cross-platform profiler #1321

The EventPipeProfiler cross-platform profiler #1321

Conversation

WojciechNagorski commented Nov 27, 2019 • edited Loading

adamsitnik commented Dec 16, 2019

WojciechNagorski commented Dec 16, 2019

adamsitnik commented Dec 17, 2019

WojciechNagorski commented Dec 17, 2019

WojciechNagorski commented Dec 18, 2019

adamsitnik left a comment

Choose a reason for hiding this comment

adamsitnik Dec 18, 2019

Choose a reason for hiding this comment

WojciechNagorski Dec 18, 2019

Choose a reason for hiding this comment

WojciechNagorski commented Dec 18, 2019

WojciechNagorski commented Mar 2, 2020

adamsitnik left a comment

Choose a reason for hiding this comment

adamsitnik Mar 3, 2020

Choose a reason for hiding this comment

adamsitnik Mar 3, 2020

Choose a reason for hiding this comment

adamsitnik Mar 3, 2020

Choose a reason for hiding this comment

adamsitnik Mar 3, 2020

Choose a reason for hiding this comment

adamsitnik Mar 3, 2020

Choose a reason for hiding this comment

adamsitnik left a comment

Choose a reason for hiding this comment

WojciechNagorski commented Mar 3, 2020

adamsitnik commented Mar 3, 2020

WojciechNagorski commented Mar 3, 2020

WojciechNagorski commented Nov 27, 2019 •

edited

Loading