[HotPath] attribute to control which methods are analysed

[mattwarren]

This is something I've thought about before, the idea of making the analysers only run on certain methods, as opposed to the entire source. Seems like other people are asking for this too, see this discussion, these some interesting points made in the replies.

My best idea at the moment is a [HotPath] attribute. With the assumption that if any method in the source has this applied, only analyse those methods. If no methods have [HotPath], then fall back to processing the entire source.

It could also be extended to allow [HotPath(ShowAsErrors=true)], [HotPath(ShowAsWarnings=true)], depending on whether it fails the build or not.

What does anyone else think?

[ljw1004]

My first instinct when I ran this analyzer was that I was overwhelmed by so many hits and didn't know what to do with them...

• Maybe HotPath is indeed the right thing. That was my instinct.
• I'd hope there's some way to automatically import hot-path annotations from a perf analysis run
• For a library author they actually don't really know what their hot paths are (don't know how they're going to be invoked). Not sure what they'd want here.
• I wonder about automatically disabling the analysis inside known-slow blocks like exception handlers?

[kasajian]

I don't really see how this analyzer can remain enabled all the time without this feature.

[edespong]

If #46 and #25 are in place, I think this will be simple to solve.

My suggestion is that we introduce two options:

1. A comma-separated string list of fully qualified names of the attributes considered "hot paths". This way a user can roll their own if they want (I cannot find a good public default attribute in any standard library).
2. Bool whether to exclusively use HotPath attributes or not. True would indicate that only code inside methods or classes marked with a HotPath attributes would be analyzed and everything else always ignored. False would mean that in a class where there is at least one method with a HotPath attribute, only that method would be analyzed. In classes with no such attributes, we would analyze everything.

This would give the following logic:

if there is at least one HotPath attribute defined:
     if the symbol is in a class with a HotPath attribute:
         run analyzers
     else if the symbol is inside a method with a HotPath attribute:
         run analyzers
     else if not exclusively use HotPath attributes:
         if the class of the symbol has no other method with a HotPath attribute:
             run analyzers

How does this sound?

[kasajian]

I don’t. I think the fundamental problem with this analyzer is that it is unusual for 99.9999% of the case. You ordinarily do not want care about heap allocations. So the only option that is necessary is one where one opts into the functionality by adorning hot path methods with a specific attribute.

---

From: Erik Edespong [email protected] Sent: Friday, March 30, 2018 1:31:47 PM To: Microsoft/RoslynClrHeapAllocationAnalyzer Cc: Kenneth Kasajian; Comment Subject: Re: [Microsoft/RoslynClrHeapAllocationAnalyzer] [HotPath] attribute to control which methods are analysed (#7)

If #46https://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/pull/46 and #25https://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/issues/25 are in place, I think this will be simple to solve.

My suggestion is that we introduce two options:

1. A comma-separated string list of fully qualified names of the attributes considered "hot paths". This way a user can roll their own if they want (I cannot find a good public default attribute in any standard library).
2. Bool whether to exclusively use HotPath attributes or not. True would indicate that only code inside methods or classes marked with a HotPath attributes would be analyzed and everything else always ignored. False would mean that in a class where there is at least one method with a HotPath attribute, only that method would be analyzed. In classes with no such attributes, we would analyze everything.

This would give the following logic:

if there is at least one HotPath attribute defined: if the symbol is in a class with a HotPath attribute: run analyzers else if the symbol is inside a method with a HotPath attribute: run analyzers else if not exclusively use HotPath attributes: if the class of the symbol has no other method with a HotPath attribute: run analyzers

How does this sound?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/issues/7#issuecomment-377616210, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACRjnYLSW7mDR9KVzhQg2aCXAF_-oUv3ks5tjpYzgaJpZM4Dtk-e.

[edespong]

There is now a test implementation that solves this issue here. I will create a pull request if/when #46 and #22 are pulled.

[mjsabby]

@edespong still taking a look, but looks good to me so far. I'll try out your build and see if there is any usability improvements we can make. My biggest concerns continues to be the VS dependency. Anyhow, you say it is unavoidable so that's that.

[edespong]

@mjsabby That depends on which projects you are referring to. I did a quick check with Resharpers "Optimize References", and there are things to improve. In my PRs, there are 4 projects in the solution:

ClrHeapAllocationAnalyzer ClrHeapAllocationAnalyzer.Common ClrHeapAllocationAnalyzer.Vsix ClrHeapAllocationAnalyzer.Test

The last two currently have Microsoft.VisualStudio references, but I think everything in Test except for Microsoft.VisualStudio.QualityTools.UnitTestFramework can be removed. Of course, we can remove the last one as well if another test framework is selected.

For the Vsix project, it would not be possible to remove the VS dependencies if we want to keep the connection to the settings and settings store.

I can look into removing all non-used dependencies the coming week.