RoslynClrHeapAllocationAnalyzer icon indicating copy to clipboard operation
RoslynClrHeapAllocationAnalyzer copied to clipboard

Does this work with 2017 now?

Open sebas77 opened this issue 7 years ago • 20 comments

If so, how to install it?

sebas77 avatar Dec 02 '17 11:12 sebas77

I'm wondering this too

abergs avatar Dec 04 '17 09:12 abergs

Downloading and installing v1.5 from https://github.com/mjsabby/RoslynClrHeapAllocationAnalyzer/releases is working for me on VS 2017 (15.5)

edespong avatar Dec 15 '17 13:12 edespong

I think this project is dead?

niemyjski avatar Dec 21 '17 17:12 niemyjski

1.5 appears to work. This projects shouldn't really die as there is no free alternative (Resharper is the only alternative solution) and is quite fundamental for a coder who values optimizations.

sebas77 avatar Dec 22 '17 18:12 sebas77

This projects shouldn't really die

MarcoRossignoli avatar Dec 27 '17 09:12 MarcoRossignoli

it has been confirmed that it still in progress by the author. Now I need https://github.com/segrived/Msiler view back as well!

sebas77 avatar Dec 27 '17 13:12 sebas77

@sebas77 where do you see it's still in progress by the author(s)?

niemyjski avatar Dec 27 '17 14:12 niemyjski

He tweeted it https://twitter.com/mjsabby/status/944297432149508096

On Wed, 27 Dec 2017, 14:13 Blake Niemyjski, [email protected] wrote:

@sebas77 https://github.com/sebas77 where do you see it's still in progress by the author(s)?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/issues/33#issuecomment-354119548, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5s49ngD-u7LEkntUp51o3F7aS4ZPGtks5tElCGgaJpZM4QzRna .

sebas77 avatar Dec 27 '17 15:12 sebas77

Sorry for the lack of progress here, we're trying to find a team that can own this so we can make it better. But for now I'm going to power through and get an update out with fixes that have gone through.

mjsabby avatar Mar 11 '18 05:03 mjsabby

@mjsabby If you have some clear ideas on what needs to be done, I can contribute some time each week. I can be contacted through jlennox at g-mail. Thanks for the work!

edit: I made a fast project to experiment with https://github.com/jlennox/DisposableVisualizer

jlennox avatar Mar 11 '18 15:03 jlennox

@mjsabby i'm not a Roslyn expert yet, but as @jlennox i could help in my spare time, let me know. Thank's for great work!With span revolution advices will be very useful.

MarcoRossignoli avatar Mar 12 '18 07:03 MarcoRossignoli

Hey all, just wondering how the latest work has been going. Anyone have any updates on when to expect a new version? Nuget package is over 2 years old now :(

phillijw avatar Jul 06 '18 18:07 phillijw

@phillijw I've considered making a fork of this project but I'm not sure what needs to be fixed. What are the main issues you're facing, or features you feel are missing?

jlennox avatar Jul 06 '18 19:07 jlennox

I think what's missing, without which some of us cannot even use the product, is the fact that such a product assumes that you want to know about heap allocations all the time in every piece of code.

That is not the case. It's not like other rules, like, you forgot to call Dispose, or something, which you want to catch every violation of. Heap allocation is a normal thing in C#, so flagging it is kinda useless unless it's being allocated in an area of code that you care about.

What you want is to mark certain methods with an attribute and only check for heap allocations in those methods, and in those cases, actually mark the code with an error instead of a warning, so people don't ignore it.

Or, the attribute should indicate a count, the number of times allocations are expected to occur, and if the code is modified such that new allocations will occur, then it will trigger. That way if you have code with a hot-spot that's taking say 50 allocations, and you work through to minimize it down to say 5, but that's all that you can do.. there's no way to reduce it further, or you are satisfied with the number, then you should be able to say "5 is the minimum for this method", and do so by indicating it in an attribute parameter above the method that this tool will check for.

ArtGangsta avatar Jul 08 '18 22:07 ArtGangsta

@ArtGangsta I like those thoughts. No promises as I'm low on free time, but I'll keep this on my mental back burner and poke at it some weekend.

jlennox avatar Jul 08 '18 23:07 jlennox

The plugin as it is, is extremely useful for game programmers. I now use jetbrains rider that supports something similar too.

On Mon, 9 Jul 2018, 00:18 Joseph Lennox, [email protected] wrote:

@ArtGangsta https://github.com/ArtGangsta I like those thoughts. No promises as I'm low on free time, but I'll keep this on my mental back burner and poke at it some weekend.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/issues/33#issuecomment-403325041, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5s46Civsn2zFBkEcNu-qoUVENYdS4Tks5uEpMpgaJpZM4QzRna .

sebas77 avatar Jul 09 '18 07:07 sebas77

@ArtGangsta There is a working version in my fork for rougly what you want, here. It is tracked by the issue #7 in this repo, but is dependent on two other pull requests being accepted.

As I have not found any fitting attribute in class libraries, I think that the attribute needs to be configurable in some way. The alternative is that we create a new package with just the attribute which can then be referenced by the projects that needs it.

The branch does currently not solve the "count-allowed-per-method", but I think that could be quite trivial to solve.

edespong avatar Jul 11 '18 14:07 edespong

@edespong I imagined it would be handled with a pragma or pragma-like comment (which is something Resharper does).

Example of a Resharper "pragma-like" hanging out with pragmas:

#pragma warning disable IDE0007 // Use implicit type
#pragma warning disable IDE1006 // Naming Styles
// ReSharper disable once CheckNamespace

Some thoughts: Syntax to disable/enable checking in a file:

// AllocationAnalyzer disable
// AllocationAnalyzer enable

Syntax to set/remove a limit between two points:

// AllocationAnalyzer limit 10
// AllocationAnalyzer limit disable

I'm not sure how easy it would be to associate your current analysis between two comments.

jlennox avatar Jul 11 '18 15:07 jlennox

I was thinking of combining a few things:

  1. Have settings in VS for the developer experience
  2. Provide code-level hints as alluded to in #7

For 2. I have the following idea:

    using System;

    [AttributeUsage(AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = true, Inherited = false)]
    public class PerformanceContractAttribute : Attribute
    {
        public DiagnosticType Diagnostic { get; set; }

        public AllocationKind AllowedAllocations { get; set; }
    }

    public enum DiagnosticType
    {
        Warning,
        Error,
        Notify
    }

    [Flags]
    public enum AllocationKind
    {
        None,
        Boxing,
        ImplicitArrayAllocation,
        Captures,
        Explicit,
        Enumerator,
        Delegate,
        All = Boxing | ImplicitArrayAllocation | Captures | Explicit | Enumerator | Delegate // useful when the analyzer setting is set to complain about everything and you still need an escape hatch
    }

    public class Foo
    {
        [PerformanceContract(AllowedAllocations = AllocationKind.Explicit | AllocationKind.Delegate)]
        public void RegularMethod()
        {
        }


        [PerformanceContract(AllowedAllocations = AllocationKind.None, Diagnostic = DiagnosticType.Error)]
        public void SuperHotMethod()
        {
        }

        [PerformanceContract(AllowedAllocations = AllocationKind.All)]
        public void InitializationOrStartupMethod()
        {
        }
    }

When coupled with @edespong's VS options feature we can basically let the user decide/override. However when an attribute on a method is present it could be used in VS AND in the build system as part of continuous integration.

So for example in a csproj, you would say:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <HeapAllocationAnalyzer>PerformanceContract</HeapAllocationAnalyzer>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="ClrHeapAllocationAnalyzer" / Version="1.*" />
  </ItemGroup>

</Project>

What do you all think?

mjsabby avatar Jul 12 '18 03:07 mjsabby

I like the principle of your proposal, msjabby, better than jlennox's. It also solves one of my main issues with just having it in the options, and that is the ability to configure for use in a CI pipeline.

I do, however, think that there is a value in jlennox point about having an accepted baseline of x allocations, which could either be solved by an int on PerformanceContractAttribute, making it very explicit with an int for every allocation kind, or possibly some other way. An alternative could be to not do anything and force the user to explicitly ignore all known allocations.

I do not know if it is desirable to be able to specify Diagnostic per AllocationKind, something that is not considered above, but I do not think that it is a dealbreaker.

I think that most of the code needed for the proposal is already scattered around a number of branches in my fork, so it should be relatively easy to get a working proposal up and running. If no one else grabs this, I will take a stab when I get back to a proper development environment in middle August.

edespong avatar Jul 22 '18 06:07 edespong