4

My job uses SonarQube as part of the CI/CD quality gate. One of the "Maintainability" issues (severity: medium) raised is the CA-1822: Mark members as static.

The link explains the added value of such change, but I am wondering: applying this change more than, say, 50 times - may not introduce memory issues due the fact that data necessary only for a specific point in time - will not be kept for the whole lifetime of the application?

Let's simplify, for the sake of the question: Suppose we have

class A
{
    public int N1 {get; set;}
    public string S1 {get; set;}

    public A(int n)
    {
       N1 = NumHelper(n);
    }

    private int NumHelper(int n)
    {
        return Random.Shared.Next(0, 10) * n;
    }
}

And say I get CA-1822 on member NumHelper. Putting aside that this is absolutely negligible for the scenario above, if we will be applying such change tens or hundreds of times in the code base - the memory now required to keep these static members - once released after GC in the past - will never increase memory consumption in any significant manner?

In other words: non static class members (fields, methods) will not remain allocated forever. They will be GCed and the memory used for them released at some time. Changing (some of) them to static does not change this?

Improve this question
1
  • 2
    "non static class members (fields, methods) will not remain allocated forever." - what memory do you think is allocated for them (e.g. as part of class instances) that could be deallocated? Commented Jul 16 at 18:53

1 Answer 1

Reset to default
15

Garbage collection deals with objects, or, in other words, instances of specific types. GC doesn't care nor deal with methods, unless you store a method as an object (such as an instance of Func<T>).

Similarly, CA-1822 has nothing to do about memory consumption, but about an extra check that is done at runtime for non-static methods. When a method is static, it can be called directly—it will always exist. An instance method, however, being bound to an instance, requires this instance to exist. Which means that the runtime should first start by checking the existence of this instance. Making methods static is about avoiding this extra check.

Putting aside that this is absolutely negligible for the scenario above

Sort of. This is one of the drawbacks of some CA rules—focusing your attention on nanosecond optimizations, as if this actually matters for the average business code.

Note, however, that unlike CA1860 that invites you to use Count > 0 on collections, instead of Any() just to avoid an extra call, at a cost of code becoming less clear, CA-1822 is actually a good thing in terms of code explicitness. After all, if a given method doesn't use any instance methods (and is not part of an interface or a base class), then mark it static—not as a part of a nanosecond optimization, but to make it clear.

Hypothetically, assuming I get a similar warning for field members (not methods) (say it makes sense) - if I start marking fields as static - in this case - my assumption above holds - in that more memory will be held over time without being released ?

It depends.

Imagine a class A with an instance field x, and two objects, a1 and a2, being initialized, and then left for GC.

After initialization, you'll end up with x using twice the memory: once per instance. At some moment, GC will release this memory.

Imagine, now, that x is static.

After initialization, you'll have half the memory used compared to the previous case.

But it will not be released.

Is this a good or a bad thing? It depends. There are cases where you absolutely want GC to free everything. And then there are cases where it is much more interesting to use memory once, and not multiplied by the number of instances of the object.

Here again, don't overthink it in terms of performance or memory consumption. If the field makes sense as instance field, make it instance field. If it is shared by all instances, make it static.

Improve this answer
8
  • Thanks! Hypothetically, assuming I get CA-1822 for field members (not methods) (say it makes sense) - if I start marking fields as static - in this case - my assumption above holds - in that more memory will be held over time without being released ? Commented Jul 16 at 9:23
  • 1
    CA-1822 is actually a good thing in terms of code explicitness - I agree in principle. Specifically here - it's probably me that frowns eyebrows on scenarios where I declare a class member where I know beforehand that it will not access any members in the class. Sounds I should not be declaring that (static) member in that class at all. Another topic. Commented Jul 16 at 9:28
  • 1
    "An instance method ... means that the runtime should first start by checking the existence of this instance. Making methods static is about avoiding this extra check." --- This little nugget has completely slipped my mind for a decade. I always dismiss the CA-1822 suggestion as being some unnecessary box-ticking exercise, but it actually has value. Who knew? (not me, obviously) Commented Jul 16 at 17:50
  • 1
    "the runtime should first start by checking the existence of this instance" - I doubt that the check matters, I would expect any reasonable modern compiler/vm to optimise this away when it knows that a variable has a value instead of being null. Relevant overhead probably is only introduced if the method dispatch is dynamic. The major point really is the code clarity - and having to write (and execute) code that explicitly creates an instance where none would be needed. The existence check part is rather insignificant. Commented Jul 16 at 19:10
  • 4
    Great explanation! I found that there is one edge case: It makes sense to keep a method non-static if the fact that it (currently) does not use instance data is an implementation detail that might change in the future. Simple example on a User class: public bool IsAdmin => true; // we don't support user roles in v1 but probably will in v2. Commented Jul 16 at 19:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.