CODEWITHSUNDEEP
c#performancearraysmemory-leaksmemory-management
hb.
hb.

Reputation: 1765

Avoiding array duplication

According to [MSDN: Array usage guidelines](http://msdn.microsoft.com/en-us/library/k2604h5s(VS.71).aspx):

Array Valued Properties

You should use collections to avoid code inefficiencies. In the following code example, each call to the myObj property creates a copy of the array. As a result, 2n+1 copies of the array will be created in the following loop.

[Visual Basic]

Dim i As Integer
For i = 0 To obj.myObj.Count - 1
   DoSomething(obj.myObj(i))
Next i

[C#]
for (int i = 0; i < obj.myObj.Count; i++)
      DoSomething(obj.myObj[i]);

Other than the change from myObj[] to ICollection myObj, what else would you recommend? Just realized that my current app is leaking memory :(

Thanks;

EDIT: Would forcing C# to pass references w/ ref (safety aside) improve performance and/or memory usage?

Upvotes: 2

Views: 361

Answers (5)

mmmmmmmm
mmmmmmmm

Reputation: 15513

Just as a hint for those who haven't use the ReadOnlyCollection mentioned in some of the answers:

[C#]

class XY
{
  private X[] array;

  public ReadOnlyCollection<X> myObj
  {
    get
    {
      return Array.AsReadOnly(array);
    }
  }
}

Hope this might help.

Upvotes: 2

Ahmed
Ahmed

Reputation: 7238

myobj will not create new item unless you explicitly create one. so to make better memory usage I recommend to use private collection (List or any) and expose indexer which will return the specified value from the private collection

Upvotes: 0

SteinNorheim
SteinNorheim

Reputation: 2207

It will not make copies of the array unless you make it do so. However, simply passing the reference to an array privately owned by an object has some nasty side-effects. Whoever receives the reference is basically free to do whatever he likes with the array, including altering the contents in ways that cannot be controlled by its owner.

One way of preventing unauthorized meddling with the array is to return a copy of the contents. Another (slightly better) is to return a read-only collection.

Still, before doing any of these things you should ask yourself if you are about to give away too much information. In some cases (actually, quite often) it is even better to keep the array private and instead let provide methods that operate on the object owning it.

Upvotes: 0

&#216;yvind Skaar
&#216;yvind Skaar

Reputation: 1840

Whenever I have properties that are costly (like recreating a collection on call) I either document the property, stating that each call incurs a cost, or I cache the value as a private field. Property getters that are costly, should be written as methods. Generally, I try to expose collections as IEnumerable rather than arrays, forcing the consumer to use foreach (or an enumerator).

Upvotes: 1

Marc Gravell
Marc Gravell

Reputation: 1062560

No, it isn't leaking memory - it is just making the garbage collector work harder than it might. Actually, the MSDN article is slightly misleading: if the property created a new collection every time it was called, it would be just as bad (memory wise) as with an array. Perhaps worse, due to the usual over-sizing of most collection implementations.

If you know a method/property does work, you can always minimise the number of calls:

var arr = obj.myObj; // var since I don't know the type!
for (int i = 0; i < arr.Length; i++) {
  DoSomething(arr[i]);
}

or even easier, use foreach:

foreach(var value in obj.myObj) {
  DoSomething(value);
}

Both approaches only call the property once. The second is clearer IMO.

Other thoughts; name it a method! i.e. obj.SomeMethod() - this sets expectation that it does work, and avoids the undesirable obj.Foo != obj.Foo (which would be the case for arrays).

Finally, Eric Lippert has a good article on this subject.

Upvotes: 5

Related Questions