-
Notifications
You must be signed in to change notification settings - Fork 63
Does this work with 2017 now? #33
Comments
I'm wondering this too |
Downloading and installing v1.5 from https://github.com/mjsabby/RoslynClrHeapAllocationAnalyzer/releases is working for me on VS 2017 (15.5) |
I think this project is dead? |
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. |
This projects shouldn't really die |
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 where do you see it's still in progress by the author(s)? |
He tweeted it https://twitter.com/mjsabby/status/944297432149508096
…On Wed, 27 Dec 2017, 14:13 Blake Niemyjski, ***@***.***> 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
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA5s49ngD-u7LEkntUp51o3F7aS4ZPGtks5tElCGgaJpZM4QzRna>
.
|
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 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 |
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 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? |
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 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. |
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, ***@***.***> 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
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA5s46Civsn2zFBkEcNu-qoUVENYdS4Tks5uEpMpgaJpZM4QzRna>
.
|
@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 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: // 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. |
I was thinking of combining a few things:
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? |
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 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. |
If so, how to install it?
The text was updated successfully, but these errors were encountered: