Gendarme.Rules.Performance

Gendarme's performance rules are located in the Gendarme.Rules.Performance.dll assembly. Latest sources are available from anonymous SVN (http://anonsvn.mono-project.com/viewcvs/trunk/mono-tools/gendarme/rules/Gendarme.Rules.Performance/).

Rules

AvoidLargeNumberOfLocalVariablesRule

This rule warns when the number of local variables exceed a maximum value (default is 64). Having a huge amount of local variables makes it hard to generate code that perform well and, likely, makes the code harder to understand.

Notes

  • This rule is available since Gendarme 2.0

AvoidLargeStructureRule

This rule warns when a structure is larger than a a maximum value (default to 16 bytes). Large structure can cause performance problems since they are value types and as such are copied by copying the whole value (size dependent) while reference types can be copied by only copying its reference (constant size). If the structure cannot be reduced in size (e.g. by removing calculated fields) then it should be turned into a class.

Bad example:

public struct BigArgb {
    long a, r, g, b;
}

Good example:

public class BigArgb {
    long a, r, g, b;
}

Notes

  • This rule is available since Gendarme 2.0

AvoidRepetitiveCastsRule

This rule warns multiple casts are done on the same value, for the same type. Casts are expensive so reducing them, by changing the logic or caching the casted value, can help performance.

Bad example:

foreach (object o in list) {
    // first cast (is)
    if (o is ICollection) {
        // second cast (as) if item implements ICollection
        Process (o as ICollection);
    }
}

Good example:

foreach (object o in list) {
    // a single cast (as) per item
    ICollection c = (o as ICollection);
    if (c != null) {
        SingleCast (c);
    }
}

Bad example:

// first cast (is):
if (o is IDictionary) {
    // second cast if item implements IDictionary:
    Process ((IDictionary) o);
    // first cast (is):
    } else if (o is ICollection) {
        // second cast if item implements ICollection:
        Process ((ICollection) o);
    }

Good example:

// a single cast (as) per item
IDictionary dict;
ICollection col;
if ((dict = o as IDictionary) != null) {
    SingleCast (dict);
    } else if ((col = o as ICollection) != null) {
        SingleCast (col);
    }

Notes

  • This rule is available since Gendarme 2.0

AvoidReturningArraysOnPropertiesRule

This rule check properties that returns arrays. Such properties can be dangerous, because the caller doesn't know whether it access the original (internal) array of the instance or if it gets a copy (clone) of the original. A solution is to turn such properties into methods and always return a copy of the array. Another is to use a proerty that returns a read-only collection.

Bad example:

public byte[] Foo {
    get {
        // return the data inside the instance
        return foo;
    }
}
 
public byte[] Bar {
    get {
        // return a copy of the instance's data
        return (byte[]) bar.Clone ();
    }
}

Good example:

public byte[] GetFoo ()
{
    return (byte[]) foo.Clone ();
}
 
public byte[] GetFoo ()
{
    return (byte[]) bar.Clone ();
}

AvoidTypeGetTypeForConstantStringsRule

This rule warns when a method use Type.GetType(string) with a constant string. Such calls requires reflection in order to return a Type instance and, for known types, can be replaced with a much faster typeof(x).

Bad example:

booleanType = Type.GetType ("System.Boolean");

Good example:

booleanType = typeof (bool);

Notes

  • This rule is available since Gendarme 2.0

AvoidUncalledPrivateCodeRule

This rule will check for non externally visible methods that are never called. The rule will warn you if a private method isn't called in it's declaring type or if an internal method doesn't have any callers in the assembly or isn't invoked by the runtime or a delegate.

Bad example:

public class MyClass {
    private void MakeSuff ()
    {
        // ...
    }
    
    public void Method ()
    {
        Console.WriteLine ("Foo");
    }
}

Good example (removing unused code):

public class MyClass {
    public void Method ()
    {
        Console.WriteLine ("Foo");
    }
}

Good example (use the code):

public class MyClass {
    private void MakeSuff ()
    {
        // ...
    }
    
    public void Method ()
    {
        Console.WriteLine ("Foo");
        MakeSuff ();
    }
}

AvoidUninstantiatedInternalClassesRule

This rule will check for all internal types and look if they are being used inside the assembly. Such types are often leftover (dead code) or debugging/testing code and is not required. However in some case the types might by needed, e.g. when accessed thru reflection or if the [InternalsVisibleTo] attribute is used on the assembly.

Bad example:

// defined, but never instantiated
internal class MyInternalClass {
    // ...
}
 
public class MyClass {
    static void Main ()
    {
        // ...
    }
}

Good example:

internal class MyInternalClass {
    // ...
}
 
public class MyClass {
    static void Main ()
    {
        MyInternalClass c = new MyInternalClass ();
        // ...
    }
}

AvoidUnneededCallsOnStringRule

This rule detects when some methods, like Clone(), Substring(0), ToString() or ToString(IFormatProvider), are being called on a string instance. Since every case returns the exact same reference the extra call(s) only slow downs performance.

Bad example:

public void PrintName (string name)
{
    Console.WriteLine ("Name: {0}", name.ToString ());
}

Good example:

public void PrintName (string name)
{
    Console.WriteLine ("Name: {0}", name);
}

Notes

  • Prior to Gendarme 2.0 this rule was more limited and named AvoidToStringOnStringsRule

AvoidUnneededFieldInitializationRule

This rule looks for constructors that assign fields to their default value (e.g. 0 for an integer, null for an object or a string). Since the CLR zeroize all values there is no need, under most circumstances, to assign default values. Doing so only adds size to source code and in IL.

Bad example:

public class Bad {
    int i;
    string s;
    
    public Bad ()
    {
        i = 0;
        s = null;
    }
}

Good example:

public class Good {
    int i;
    string s;
    
    public Good ()
    {
        // don't assign 'i' since it's already 0
        // but we might prefer to assign a string to String.Empty
        s = String.Empty;
    }
}

Notes

  • This rule is available since Gendarme 2.2

AvoidUnneededUnboxingRule

This rule check methods that unbox several time the same value type. Since each unbox operation is costly, as it involves a copy, the code should be rewritten to minimize unbox operations. E.g. using a local variable of the right value type should remove need for more than one unbox instruction per variable.

Bad example:

public struct Message {
    private int msg;
    private IntPtr hwnd, lParam, wParam, IntPtr result;
    
    public override bool Equals (object o)
    {
        bool result = (this.msg == ((Message) o).msg);
        result &= (this.hwnd == ((Message) o).hwnd);
        result &= (this.lParam == ((Message) o).lParam);
        result &= (this.wParam == ((Message) o).wParam);
        result &= (this.result == ((Message) o).result);
        return result;
    }
}

Good example:

public struct Message {
    private int msg;
    private IntPtr hwnd, lParam, wParam, IntPtr result;
    
    public override bool Equals (object o)
    {
        Message msg = (Message) o;
        bool result = (this.msg == msg.msg);
        result &= (this.hwnd == msg.hwnd);
        result &= (this.lParam == msg.lParam);
        result &= (this.wParam == msg.wParam);
        result &= (this.result == msg.result);
        return result;
    }
}

Notes

  • This rule is available since Gendarme 2.0

AvoidUnsealedConcreteAttributesRule

This rule is used to warn the developer if both unsealed and concrete (not abstract) attribute types are defined in the assembly. If you want other attributes to be able to derive from your attribute class, make it abstract. Otherwise, make them sealed to improve the performance.

Bad example:

[AttributeUsage (AttributeTargets.All)]
public class BadAttribute : Attribute {
}

Good example (sealed):

[AttributeUsage (AttributeTargets.All)]
public sealed class SealedAttribute : Attribute {
}

Good example (abstract and sealed):

[AttributeUsage (AttributeTargets.All)]
public abstract class AbstractAttribute : Attribute {
}
 
[AttributeUsage (AttributeTargets.All)]
public sealed class ConcreteAttribute : AbstractAttribute {
}

Notes

  • Before Gendarme 2.0 this rule was named AvoidUnsealedAttributesRule.

AvoidUnsealedUninheritedInternalTypeRule

Since a JIT is able to apply more optimization to sealed types this rule checks for non-visible (outside the current assembly) and non-sealed types to find the ones that no other types inherit from.

Bad example:

// this one is correct since MyInheritedStuff inherits from this class
internal class MyBaseStuff {
}
 
// this one is bad, since no other class inherit from MyConcreteStuff
internal class MyInheritedStuff : MyBaseStuff {
}

Good example:

// this one is correct since the class is abstract
internal abstract class MyAbstractStuff {
}
 
// this one is correct since the class is sealed
internal sealed class MyConcreteStuff : MyAbstractStuff {
}

Notes

  • This rule is available since Gendarme 2.0 and, before 2.2, was named AvoidUnsealedUninheritedInternalClassesRule

AvoidUnusedParametersRule

This rule is used to ensure that all parameters in a method signature are being used. The rule wont report a defect against the following:

  • Methods that are referenced by a delegate;
  • Methods used as event handlers;
  • Abstract methods;
  • Virtual or overriden methods;
  • External methods (e.g. p/invokes)


Bad example:

public void MethodWithUnusedParameters (IEnumerable enumerable, int x)
{
    foreach (object item in enumerable) {
        Console.WriteLine (item);
    }
}

Good example:

public void MethodWithUsedParameters (IEnumerable enumerable)
{
    foreach (object item in enumerable) {
        Console.WriteLine (item);
    }
}

AvoidUnusedPrivateFieldsRule

This rule checks all private fields inside each type to see if some of them are not being used. This could be a leftover from debugging or testing code or a more serious typo where a wrong field is being used. In any case this makes the type bigger than it needs to be and this can affect performance when a large number of instances exists.

Bad example:

public class Bad {
    int level;
    bool b;
    
    public void Indent ()
    {
        level++;
        #if DEBUG
        if (b) Console.WriteLine (level);
        #endif
    }
}

Good example:

public class Good {
    int level;
    #if DEBUG
    bool b;
    #endif
    
    public void Indent ()
    {
        level++;
        #if DEBUG
        if (b) Console.WriteLine (level);
        #endif
    }
}

CompareWithEmptyStringEfficientlyRule

This rule will check the comparison of a string variable with "" or String.Empty. This promote the use of the String.Length property. Another way to check for an empty string (and a null one at the same time) is available since the version 2.0 of the framework by using the static method String.IsNullOrEmpty.

Bad example:

public void SimpleMethod (string myString)
{
    if (myString.Equals (String.Empty)) {
    }
}

Good example:

public void SimpleMethod (string myString)
{
    if (myString.Length == 0) {
    }
}

ConsiderCustomAccessorsForNonVisibleEventsRule

This rule looks for non-visible events to see if their add/remove accessors are the default ones. The default, compiler generated, accessor are marked as synchronized which means that the runtime will them between Monitor.Enter and Monitor.Exit calls. This is the safest approach unless, for non-visible events, you have a performance bottleneck around the events. In this case you should review if your code needs the locks or if you can provide some alternatives to them.

Bad example:

private event EventHandler<TestEventArgs> TimeCritical;

Good example:

static object TimeCriticalEvent = new object ();
EventHandlerList events = new EventHandlerList ();
 
private event EventHandler<TestEventArgs> TimeCritical {
    add {
        events.AddHandler (TimeCriticalEvent, value);
    }
    remove {
        events.AddHandler (TimeCriticalEvent, value);
    }
}

Notes

  • This rule is available since Gendarme 2.0

DoNotIgnoreMethodResultRule

This rule detects when some code doesn't use the return value of a method call. Since any returned object potentially requires memory allocations this impacts performance. Furthermore this often indicates that the code might not be doing what is expected. This is seen frequently on string where people forgets their immutability. There are some special cases, e.g. StringBuilder, where some methods returns the current instance (to chain calls). The rule will ignore those well known cases.

Bad example:

public void GetName ()
{
    string name = Console.ReadLine ();
    // a new trimmed string is created by never assigned to anything
    // but name itself is unchanged
    name.Trim ();
    Console.WriteLine ("Name: {0}", name);
}

Good example:

public void GetName ()
{
    string name = Console.ReadLine ();
    name = name.Trim ();
    Console.WriteLine ("Name: {0}", name);
}

ImplementEqualsTypeRule

This rule looks for types that overrides Object.Equals(object) and that do not provide a Equals(x) overload for their own type. Such an overload removes the need to cast the object to the correct type. For value types this also remove the costly boxing operations. Assemblies targeting the framework 2.0 (and later) should also have the type implement System.IEquatable<T>.

Bad example:

public class Bad {
    public override bool Equals (object obj)
    {
        return base.Equals (obj);
    }
    
    public override int GetHashCode ()
    {
        return base.GetHashCode ();
    }
}

Good example:

// IEquatable<T> is only available since
// version 2.0 of the .NET framework
public class Good : IEquatable<Good> {
    public override bool Equals (object obj)
    {
        return (obj as Good);
    }
    
    public bool Equals (Good other)
    {
        return (other != null);
    }
    
    public override int GetHashCode ()
    {
        return base.GetHashCode ();
    }
}

Notes

  • This rule is available since Gendarme 2.0

MathMinMaxCandidateRule

This rule checks methods that seems to include code duplicating Math.Min or Math.Max functionality. The JIT can inline those methods and provide better performance compared to custom, inline, implementations.

Bad example:

int max = (a > b) ? a : b;

Good example:

int max = Math.Max (a, b);

Notes

  • This rule is available since Gendarme 2.0

OverrideValueTypeDefaultsRule

This rule checks all value types, except enumerations, to see if they use the default implementation of Equals(object) and GetHashCode() methods. While ValueType implementations work for any value type they do so at the expense of performance (since it must reflect all fields). You can easily override both methods with much faster code since you know all meaningful fields inside your structure. At the same time you should also provide, if your language allows it, operator overloads for equality (op_Equality, ==) and inequality (op_Inequality, !=).

Bad example:

public struct Coord {
    int X, Y, Z;
}

Good example:

public struct Coord {
    int X, Y, Z;
    
    public override bool Equals (object obj)
    {
        if (obj == null) {
            return false;
        }
        Coord c = (Coord)obj;
        return ((X == c.X) && (Y == c.Y) && (Z == c.Z));
    }
    
    public override int GetHashCode ()
    {
        return X ^ Y ^ Z;
    }
    
    public static bool operator == (Coord left, Coord right)
    {
        return left.Equals (right);
    }
    
    public static bool operator != (Coord left, Coord right)
    {
        return !left.Equals (right);
    }
}

Notes

  • This rule is available since Gendarme 2.0

PreferCharOverloadRule

This rule looks for calls into String methods that use Stringparameters when a Char parameter could have been used. Using the Char-based method overload (ordinal comparison) is faster than the String-based one (culture-sensitive comparison). Note that .NET 4 will do a breaking change and will use ordinal comparison by default on String. If you need a culture-sensitive comparison use an overload that allow you to specify a StringComparison. This rule checks specifically for IndexOf, LastIndexOf and Replace to ensure there is a performance gain (other methods could vary).

Bad example:

if (s.IndexOf (":") == -1) {
    Console.WriteLine ("no separator found");
}

Good example:

if (s.IndexOf (':') == -1) {
    Console.WriteLine ("no separator found");
}

Notes

  • This rule is available since Gendarme 2.4

PreferLiteralOverInitOnlyFieldsRule

This rule looks for InitOnly fields (readonly in C#) that could be turned into Literal (const in C#) because their value is known at compile time. Literal fields don't need to be initialized (i.e. they wont show up or add a static constructor to the type) resulting in less code and the value (not a reference to the field) will be directly used in the IL (but this can be a double-edged sword).

Bad example:

public class ClassWithReadOnly {
    static readonly int One = 1;
}

Good example:

public class ClassWithConst
{
    const int One = 1;
}

Notes

  • This rule is available since Gendarme 2.2

RemoveUnneededFinalizerRule

This rule looks for types that have empty finalizer (a.k.a. destructor in C# or Finalize method). Finalizer that simply nullify fields are considered as empty since this does not help the garbage collection. You should remove the empty finalizer to reduce the GC involvement (and get better performance) when the object instances are freed.

Bad example (empty):

class Bad {
    ~Bad ()
    {
    }
}

Bad example (only nullify fields):

class Bad {
    object o;
    
    ~Bad ()
    {
        o = null;
    }
}

Good example:

class Good {
    object o;
}

Notes

  • Prior to Gendarme 2.2 this rule was named EmptyDestructorRule

RemoveUnusedLocalVariablesRule

This rule looks for unused local variables inside methods. This can leads to larger code (IL) size and longer JIT time, but note that some compiler while optimizing can remove the locals so they won't be reported even if you can still see them in the source code. This could also be a typo in the source were a value is assigned to the wrong variable.

Bad example:

bool DualCheck ()
{
    bool b1 = true;
    bool b2 = CheckDetails ();
    if (b2) {
        // typo: a find-replace changed b1 into b2
        b2 = CheckMoreDetails ();
    }
    return b2 && b2;
}

Good example:

bool DualCheck ()
{
    bool b1 = true;
    bool b2 = CheckDetails ();
    if (b2) {
        b1 = CheckMoreDetails ();
    }
    return b1 && b2;
}

Notes

  • This rule is available since Gendarme 2.0

UseIsOperatorRule

This rule checks each method to look for a complex cast operation (e.g. a aswith a null check) that could be simplified using the is operator (C# syntax). Note: in some case a compiler, like [g]mcs, can optimize the code and generate IL identical to a is operator. In this case the rule will not report an error even if you could see one while looking the at source code.

Bad example:

bool is_my_type = (my_instance as MyType) != null;

Good example:

bool is_my_type = (my_instance is MyType);

Notes

  • This rule is available since Gendarme 2.0

UseStringEmptyRule

This rule checks for methods that are using the literal "" instead of the String.Empty field. You'll get slighly better performance by using String.Empty. Note that in some case, e.g. in a switch/case statement, you cannot use a field, so "" must be used instead of String.Empty.

Bad example:

string s = "";

Good example:

string s = String.Empty;

UseSuppressFinalizeOnIDisposableTypeWithFinalizerRule

This rule catches a common problem when a type has a finalizer (called a destructor in C#) and implements System.IDisposable. In this case the Dispose method should call System.GC.SuppressFinalize to avoid the finalizer to be, needlessly, called.

Bad example:

class Test: SomeClass, IDisposable {
    ~Test ()
    {
        if (ptr != IntPtr.Zero) {
            Free (ptr);
        }
    }
    
    public void Dispose ()
    {
        if (ptr != IntPtr.Zero) {
            Free (ptr);
        }
    }
}

Good example:

class Test: SomeClass, IDisposable {
    ~Test ()
    {
        if (ptr != IntPtr.Zero) {
            Free (ptr);
        }
    }
    
    public void Dispose ()
    {
        if (ptr != IntPtr.Zero) {
            Free (ptr);
        }
        GC.SuppressFinalize (this);
    }
}

Notes

  • Prior to Gendarme 2.2 this rule was named IDisposableWithDestructorWithoutSuppressFinalizeRule

UseTypeEmptyTypesRule

This rule looks for methods that creates an empty (zero sized) array of System.Type. This value is so often required by the framework API that the System.Type includes a EmptyTypes field. Using this field avoids the memory allocation (and GC tracking) of your own array.

Bad example:

ConstructorInfo ci = type.GetConstructor (new Type[0]);

Good example:

ConstructorInfo ci = type.GetConstructor (Type.EmptyTypes);

Notes

  • This rule is available since Gendarme 2.0

Feedback

Please report any documentation errors, typos or suggestions to the Gendarme Google Group (http://groups.google.com/group/gendarme). Thanks!