Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support of --profiler NativeMemory command line argument #1208

Merged
merged 5 commits into from Aug 2, 2019

Conversation

@WojciechNagorski
Copy link
Collaborator

WojciechNagorski commented Jul 23, 2019

In this PR I support command line argument:

--profiler NativeMemory

@adamsitnik I wonder if NativeMemoryDiagnoser shouldn't be named NativeMemoryProfiler?
We have MemoryDiagnoser but there is also EtwProfiler and ConcurrencyVisualizerProfiler.

@adamsitnik

This comment has been minimized.

Copy link
Member

adamsitnik commented Jul 23, 2019

I wonder if NativeMemoryDiagnoser shouldn't be named NativeMemoryProfiler?

I am not sure. On the one hand, it's using EtwProfiler API to get the data. On the other, it reports the metrics and doesn't print a file path to the trace file so it's transparent to the end-user.

Does the trace file contain enough information to track the type names? Or something very useful in general? If so, you can rename it to Profiler but also print the paths to the trace files.

@WojciechNagorski

This comment has been minimized.

Copy link
Collaborator Author

WojciechNagorski commented Jul 28, 2019

I'm going to check it. If it is possible to trace the type names, I will change the name to Profiler.

@adamsitnik

This comment has been minimized.

Copy link
Member

adamsitnik commented Jul 29, 2019

I'm going to check it. If it is possible to trace the type names, I will change the name to Profiler.

Great, please let me know once you got it so we can merge this PR and release 0.11.6

@WojciechNagorski

This comment has been minimized.

Copy link
Collaborator Author

WojciechNagorski commented Jul 30, 2019

@adamsitnik I know what I have to do to track the type names, using PerfView. It requires small changes in NativeMemoryDiagnoser, but after this changes .etl file will be a little bigger because we need to save information about stack traces. Do you think it will be a useful function?

@adamsitnik

This comment has been minimized.

Copy link
Member

adamsitnik commented Jul 30, 2019

Do you think it will be a useful function?

Yes! I think that it would be nice if users could find out not only the native allocation size but also what was allocated if they open the trace file in a profiler.

@adamsitnik

This comment has been minimized.

Copy link
Member

adamsitnik commented Jul 31, 2019

@wojtpl2 please let me know when this is ready to test ;)

@WojciechNagorski

This comment has been minimized.

Copy link
Collaborator Author

WojciechNagorski commented Jul 31, 2019

Not yet but today morning it will be. I just need test it and describe how it works.

@WojciechNagorski

This comment has been minimized.

Copy link
Collaborator Author

WojciechNagorski commented Jul 31, 2019

It is ready to test.
In this test, I allocate an array of int twice and an array of Point once.
image

  • An array of int with 200 length -> 200 * 4B = 800B
  • An array of int with 150 length -> 150 * 4B = 600B
  • An array of Point (value int X and int Y) with 200 length -> 200 * 2 * 4B = 1600B

SUM 3000B
There are output from BDN:
image
image
This is confirmation of these values:
image
There are print screans from PerfView with information about types.
image
image

I didn't have to make any changes in MemoryNativeProfiler because heap session logs stack traces. Then PerfView can read type from stack trace:
image

@WojciechNagorski

This comment has been minimized.

Copy link
Collaborator Author

WojciechNagorski commented Aug 2, 2019

@adamsitnik I have a question when you want to release a new version of BDN? I would like to know how much time I have to write my blog about this topic.

Copy link
Member

adamsitnik left a comment

Thank you @wojtpl2 !

@adamsitnik adamsitnik merged commit 9caa055 into dotnet:master Aug 2, 2019
4 checks passed
4 checks passed
WIP Ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@adamsitnik

This comment has been minimized.

Copy link
Member

adamsitnik commented Aug 2, 2019

I have a question when you want to release a new version of BDN? I would like to know how much time I have to write my blog about this topic.

My plan is to finish #1188 on Monday and then we should be able to release a new version by the end of next week

@WojciechNagorski WojciechNagorski deleted the WojciechNagorski:native_memory_profiler branch Aug 2, 2019
@AndreyAkinshin AndreyAkinshin added this to the v0.11.6 milestone Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.