Fix native memory profiler

[WojciechNagorski]

This PR fixes #1427 for .Net Core and .Net Framework.

The following benchmark is problematic:

        [Benchmark]
        public string StackTrace()
        {
            var stackTrace = new StackTrace();

            return stackTrace.GetType().Name;
        }

I don't know why but new StackTrace() causes different workings of the benchmark.

.Net Core

There are allocation (propably from JIT) between ActulaWorkStart and Stop :

It is a event of JIT of WorkloadActualStop method.

ThreadID="13,340" ProcessorNumber="0" MethodID="140,712,104,538,952" ModuleID="140,712,102,993,560" MethodToken="100,665,663" MethodILSize="10" MethodNamespace="BenchmarkDotNet.Engines.EngineEventSource" MethodName="WorkloadActualStop" MethodSignature="instance void  (int64)" ClrInstanceID="8"

The following screen shows that that allocations do not come from our benchmark:

So I solved it by filtering events only from benchmark stacks. I wanted to fix it by running a benchmark twice but I got a different JIT event.

.NET Framework without fix

In this case, there are allocations between ActualWork Start and Stop.

But I don't know anything about the origin of these allocations.

.Net Framework with fix

I found a fix during analyzing the PerfView code. I don't know why but these allocations disappear after adding ClrTraceEventParser.Keywords.JittedMethodILToNativeMap

Summary

I tested if using my repository https://github.com/WojciechNagorski/NativeMemorySample. If you want to check it you need to change references to a custom build of BenchmarkDotNet.

[WojciechNagorski]

@jjolidon, @jmmorato, @Meetsch, or @pleonex Can you check if my fix works in your environment?

[jmmorato]

Hi @WojciechNagorski,

Quick test after clone your branch and replace NuGet references:

BenchmarkDotNet=v0.12.0.20200508-develop, OS=Windows 10.0.18362.418 (1903/May2019Update/19H1)
Intel Core i7-7820HQ CPU 2.90GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
[Host] : .NET Framework 4.8 (4.8.3752.0), X64 RyuJIT
ShortRun : .NET Framework 4.8 (4.8.3752.0), X64 RyuJIT

Job=ShortRun InvocationCount=1 IterationCount=3
LaunchCount=1 UnrollFactor=1 WarmupCount=3

Method	Payload	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated	Allocated native memory	Native memory leak
FileTransferPayload	10000	1.453 ms	7.577 ms	0.4153 ms	-	-	-	-	691132 B	376790 B

I'm using the following configuration:

        /// <summary>
        /// Initializes a new instance of the <see cref="BenchmarkConfig"/> class.
        /// </summary>
        public BenchmarkConfig()
        {
#if DEBUG
            AddValidator(JitOptimizationsValidator.DontFailOnError);
#endif
            AddJob(Job.ShortRun);
            AddColumnProvider(DefaultConfig.Instance.GetColumnProviders().ToArray());
            AddLogger(DefaultConfig.Instance.GetLoggers().ToArray());
            AddExporter(PlainExporter.Default);
            AddExporter(CsvExporter.Default);
            AddExporter(MarkdownExporter.Default);
            AddDiagnoser(MemoryDiagnoser.Default);
            AddDiagnoser(new NativeMemoryProfiler());
        }

[WojciechNagorski]

@jmmorato Is the result ok or not?
Can you share a source of the benchmark and csproj?

[jjolidon]

Hey,
Thanks a lot for the work on this!
I have tested with my simple reproducer and your fixes, it seems the leak is smaller but still present:
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.14393.3595 (1607/AnniversaryUpdate/Redstone1)
Intel Core i7-7820HQ CPU 2.90GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
Frequency=2835938 Hz, Resolution=352.6170 ns, Timer=TSC
.NET Core SDK=3.1.201
[Host] : .NET Core 3.1.3 (CoreCLR 4.700.20.11803, CoreFX 4.700.20.12001), X64 RyuJIT
ShortRun : .NET Core 3.1.3 (CoreCLR 4.700.20.11803, CoreFX 4.700.20.12001), X64 RyuJIT

Method	Gen 0	Gen 1	Gen 2	Allocated	Allocated native memory	Native memory leak
MyBenchmark	2.8687	0.6714	-	11.8 KB	2.23 KB	0.05 KB

(I have removed unrelated columns).
Sorry to bring in the bad news, but the bug doesn't seem to be fixed. Still, it doesn't mean your changes are necessarily useless, but there seems to be more to this issue.

[WojciechNagorski]

@jjolidon Big thanks! I will check it one more time ;)

[jjolidon]

OH WAIT, I had the wrong version restored, it would seem. Clearing the nuget cache solved that. Sorry for my mistake... Your branch fixes the leak shown by my reproducer, thanks again for the work!

BenchmarkDotNet=v0.12.0.20200508-develop, OS=Windows 10.0.14393.3595 (1607/AnniversaryUpdate/Redstone1)
Intel Core i7-7820HQ CPU 2.90GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
Frequency=2835938 Hz, Resolution=352.6170 ns, Timer=TSC
.NET Core SDK=3.1.201
[Host] : .NET Core 3.1.3 (CoreCLR 4.700.20.11803, CoreFX 4.700.20.12001), X64 RyuJIT
ShortRun : .NET Core 3.1.3 (CoreCLR 4.700.20.11803, CoreFX 4.700.20.12001), X64 RyuJIT

Job=ShortRun Force=True IterationCount=3
LaunchCount=1 WarmupCount=3

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated	Allocated native memory	Native memory leak
MyBenchmark	33.81 μs	25.66 μs	1.406 μs	2.8687	0.6714	-	11.8 KB	2.18 KB	-

[WojciechNagorski]

@jjolidon I have the same result. Thanks!
@adamsitnik It is ready to review.

[jmmorato]

Is the result ok or not?

Well... the result is not ok for me in terms of memory leak reported :P but after your fix I cannot ensure that the problem is not in my side without more investigation.

Can you share a source of the benchmark and csproj?

I'm creating a wrapper library (OpenDDSharp) over a C++ library (OpenDDS). Right now, the development environment is not easy to setup (I need to work on that), so I prefer to share with you a minimal reproducer using the already compiled NuGet package PerformanceTest.zip (You'll need to change the Benchmark.DotNet project references).

I just discovered your great diagnoser few weeks ago and start to play around with it. I'm really interested in your fix and/or improvements, so anything I could do to help you just let me know.

Thanks for your job.

[adamsitnik]

@WojciechNagorski thank you for another great PR!

I would like to make sure that it's the simplest solution before I hit the merge button. Please reply to my question.

[WojciechNagorski]

@adamsitnik I think it is ready for review.

[WojciechNagorski]

@adamsitnik Build passed

[WojciechNagorski]

I created the repo https://github.com/WojciechNagorski/NativeMemorySample that contains benchmarks that use native memory allocation.

[adamsitnik]

@WojciechNagorski LGTM, big thanks for doing this!

[WojciechNagorski]

@adamsitnik Thanks for review!