RoslynClrHeapAllocationAnalyzer icon indicating copy to clipboard operation
RoslynClrHeapAllocationAnalyzer copied to clipboard

Documentation (or, is this project dead?)

Open JasonBock opened this issue 5 years ago • 6 comments

I've been playing around with this package, and while I think it has some merit and potential, it feels woefully under-documented, and its advice is confusing.

For example, I ran across an interpolated string in my code where the analyzer raised HAA0601. The help link does nothing, nor is there any code fix available, so I really don't know how to fix this. I'm guessing it's because the value types I have in the string will get boxed? So I created a small benchmark project:

using BenchmarkDotNet.Attributes;

namespace TestFormatting
{
	[MemoryDiagnoser]
	public class FormattingAndBoxing
	{
		private readonly uint value1 = 324;
		private readonly int value2 = 43219;

		[Benchmark]
		public int FormatStringWithNoToString() =>
			$"The expected call count is incorrect. Expected: {this.value1}, received: {this.value2}.".Length;

		[Benchmark]
		public int FormatStringWithToString() =>
			$"The expected call count is incorrect. Expected: {this.value1.ToString()}, received: {this.value2.ToString()}.".Length;
	}
}

The results were....interesting:

|                     Method |     Mean |    Error |   StdDev |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------------------- |---------:|---------:|---------:|-------:|------:|------:|----------:|
| FormatStringWithNoToString | 400.7 ns | 4.240 ns | 3.759 ns | 0.0496 |     - |     - |     208 B |
|   FormatStringWithToString | 118.4 ns | 1.277 ns | 1.132 ns | 0.0687 |     - |     - |     288 B |

So the second way is faster, but it actually allocates a bit more memory? Is this correct? If so, how do I really address this analyzer's diagnostic? Or is my test faulty?

It's really down to prescriptive advice. This package needs far better documentation for it to be useful on every-day projects. I'd love to use it and continue to provide feedback, but it's unclear if this is even being maintained anymore.

JasonBock avatar Aug 18 '19 21:08 JasonBock

The project is not dead ... but at the same time the Roslyn team has decided they are going to incorporate this into some sort of Roslyn tooling. @jinujoseph / @sharwell?

mjsabby avatar Sep 14 '19 21:09 mjsabby

The Roslyn team forked this code to dotnet/roslyn-analyzers with the intent of working towards a production-ready set of performance analyzers. Allocations are only one source of observed performance problem. Ideally, we want users to be able to document "performance sensitive" sections of code with [PerformanceSensitive], which links to a documented measurement scenario and describes the constraints that developers working in the code need to be aware of. Where possible, the analyzers would help enforce those constraints during builds.

sharwell avatar Sep 16 '19 15:09 sharwell

Is there a preview of these performance analyzers yet?

stebet avatar Oct 09 '19 14:10 stebet

@stebet The analyzers are getting published here: https://dotnet.myget.org/feed/roslyn-analyzers/package/nuget/Microsoft.CodeAnalysis.PerformanceSensitiveAnalyzers

The issue tracker for that package is here: https://github.com/dotnet/roslyn-analyzers/issues

sharwell avatar Oct 09 '19 15:10 sharwell

I agree with @JasonBock. I loved this package, but even for simple things can be hard to tell what's wrong or what we can do about it. For example:

    public class Foo
    { 
        public int Id { get; set; }
    }

    public class Bar
    {
        void DoSomethingWithFoo(Foo someFoo)
        {
            var somethingWithFoo = $"We received the Foo Id {someFoo.Id}";
        }
    }

We received this warning:

HAA0601 Value type to reference type conversion causes boxing at call site (here), and unboxing at the callee-site. Consider using generics if applicable

and... what? I can't imagine a simpler code, and for some reason we need change or fix this code. A really really simple suggestion about what's happening or what we can do will be really amazing.

GitClickOk avatar Sep 01 '21 22:09 GitClickOk

So....did this ever get "moved"/"merged" to dotnet/roslyn-analyzers? I've looked at the site and I can't find any information on allocation analyzers, but I might just be missing it. If it has been moved there, I can close this comment.

JasonBock avatar Mar 08 '22 14:03 JasonBock