Improve Memory Diagnoser

[adamsitnik]

• MemoryDiagnoser should not include allocations from IterationSetup/Cleanup
• MemoryDiagnoser should not require extra run to get memory stats
• MemoryDiagnoser should not complicate the design of Engine (by prohibiting the possibility to allocate managed memory)
• MemoryDiagnoserTests should not run for more than few minutes

[adamsitnik]

@AndreyAkinshin I was able to get rid of all our Memory Diagnoser problems!

I got stupid simple idea: why do we need to run once again all benchmark iterations to get the gc stats, when we can just run one extra iteration? I can't believe it was not my design since the beginning ;)

AppDomain.Monitoring Overhead (the thing required to get allocated memory for classic .NET)? I enable it in the last iteration, so there are no iterations after it.

This has also solved the problem of Iteration Setup/Cleanup affecting the gc stats. @ig-sinicyn

And we no longer need to avoid managed allocations in Engine.

I got perf improvement x2. Our integration tests are no longer 1h ;)

[ig-sinicyn]

@adamsitnik, one-iteration measurement may be inaccurate as clr performs allocations in 8kb blocks. So, if a single iteration allocates 200 bytes you'll get 8kb in diagnoser's output (or 0kb, if there's a free space in a last allocated chunk).

[adamsitnik]

@ig-sinicyn I am aware of that. It's also less likely to trigger Gen 2 cleanup. The thing is that we usually perform billions of invocations per iteration, so it should not be a problem.

Considering all advantages (perf x2 + no iteration setup/cleanup side effects + engine can allocate) I believe that it's still the best solution.

[AndreyAkinshin]

Hooray! @adamsitnik, I love it, looks great!
About accuracy: I think, we should use the same approach which we use for basic timestamping. We know the number of operations per iteration, we know the number of GC 0/1/2 collections, we know allocation measurements. Thus, we can detect such situations and perform additional MemoryDiagnoser iterations if it's required. However, I believe that current approach will work ok for 99% of use cases.
@adamsitnik, what if we will run N extra iterations for memory diagnoser instead of one extra iteration. N can be a parameter of a diagnoser (e.g. [MemoryDiagnoser(IterationCount = 4)]) with default value N=1. It should be easy to implement + people who really need a good accuracy level for memory allocations will be able to increase it manually. In the future, we will implement automatic logic which will detect the best amount of iterations. What do you think?

I got perf improvement x2. Our integration tests are no longer 1h ;)

It's super awesome!

[adamsitnik]

@AndreyAkinshin I don't think it will be needed. If we ever face any issue I am going to fix it. Let's keep it super simple for now and see how it goes. So far I have run plenty of benchmarks after the changes and the accuracy was always 100%

[Martinnes]

Great work @adamsitnik! I'm using your code to do some performance measuring on datastructures. Currently, i do preprocessing of the data structure in a [SetupIteration] function and do actual performance test in the [Benchmark] function. However, this leaves me with a blank Allocated column, when running the tests with [MemoryDiagnoser]. Presumably because the MemoryDiagnoser no longer measures the [SetupIteration] function. Is there some way to make the [MemoryDiagnoser] include the memory used in the [SetupIteration] function - like the "old" solution? I would like to keep the setup of data structures separate from the test, but still be able to measure the memory used.

[adamsitnik]

Is there some way to make the [MemoryDiagnoser] include the memory used in the [SetupIteration] function

@Martinnes No, mostly because it was a bug. Whatever is in setup should not be measured as part of the benchmark method. Here you can read more about it: https://github.com/dotnet/performance/blob/master/docs/microbenchmark-design-guidelines.md#Setup

[Martinnes]

Thanks @adamsitnik I understand. Do you have any suggestions to how I could measure the size of the datastructure then? One example is that an array is created in the setup and the test does nothing but change values in the array

[adamsitnik]

Do you have any suggestions to how I could measure the size of the datastructure then?

Most probably the simplest way to do it would be to just add an extra benchmark that allocates the array and does nothing else. Run it once and after you get the answer, remove it, or comment out.

[adamsitnik]

Another option would be to add a custom column, but it would require much more work

[Martinnes]

Okay, I will probably just stick with rerunning the benchmarks. Thank you for the help and quick answers, it highly appreciated