CArray slows execution

Is the difference in DEBUG or RELEASE modes?  DEBUG mode will do a load of checking that the RELEASE build doesn't do.

Check these 2 links
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore/html/_core_collections.3a_.choosing_a_collection_class.asp

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dv_vcce4/html/evconImprovingTimedshCriticalCode.asp

What were you storing in the arrays when your code was array-based? What are you storing in CArray?

If the arrays don't grow or shrink, you could just dynamically allocate the array to the size you need, and not use a fixed size. Using CArray doesn't seem to add much here.

Without seeing some code showing how you were using arrays, and how you are using the CArray class, it's hard to say why the performance of CArray is so much worse.

The difference appears in the Release mode.

All arrays, there are about 30 of them, have "normal" things stored in the: Integers, double values, boolean values. I used fixed length of 5000, but sometimes all I need is 20 elements, known only at run time.

I don't know why CArray is so slow, you'd have to profile it to see where all the time is going.

Two suggestions:

1) instead of doing

int myarray[5000];

allocate the array at runtime:

int *myarray;

// get how big we need
howbig = 100;
myarray = new int[howbig];

and clean it up on exit:

delete [] myarray;

2) Try using std::vector instead of CArray, it may be more efficient

Also, note that 30 arrays of 5000 objects = 150,000 array elements.

Since you are only using plain old data, worst case is an array element is 8 bytes, to your arrays only take up 1.2 megabytes max.

I'm not sure it's worth the effort to try to optimize such a small amount of memory usage.

the arrays' size can approach the maximum limit in some cases. In fact I had cases where I indeed ran out of memory (long before the 5000 elements limit was reached). This is why I wanted to optimize the memory usage: for the cases where the arrays are indeed large.
There are also a copule of arrays that include quite large objects.

> There are also a copule of arrays that include quite large objects.

Maybe this is where the problem is. Inserting items into CArray probably causes the object to be copied twice, once as it is put on the stack for the Set() or [] call, again as CArray makes its own copy. There may also be other operations which result in these objects being copied.

If these large objects are expensive to copy, that may be where the performance hit comes from.

You could try storing pointers in the CArray and dynamically allocate the objects, although that would probably require a lot of code changes.

I didn't implement these large ogject arrays in CArray yet, since the "traditional" objects (integer, double, boolean) already created delay problems. I think implementing CArray for these large object arrays will deteriorate things more. My worry is that I am missing something, and that is what causing this slugginess. If this slow behaviour of CArray is typical, then I wont follow up on implementing it.

I just tried the following:

=============
  CArray<int> carray;
  vector<int> vec;
  int array[5000];
  int *parray = new int [5000];

  int i;

  int t1 = GetTickCount();
  for (i=0; i<50000000; i++)
  {
    array[i%5000] = array[i%5000] + i%43;
  }
  int t2 = GetTickCount();

  int t3 = GetTickCount();
  for (i=0; i<50000000; i++)
  {
    parray[i%5000] = parray[i%5000] + i%43;
  }
  int t4 = GetTickCount();

  carray.SetSize(5000);
  int t5 = GetTickCount();
  for (i=0; i<50000000; i++)
  {
    carray[i%5000] = carray[i%5000] + i%43;
  }
  int t6 = GetTickCount();

  vec.resize(5000);
  int t7 = GetTickCount();
  for (i=0; i<50000000; i++)
  {
    vec[i%5000] = vec[i%5000] + i%43;
  }
  int t8 = GetTickCount();

  CString msg;
  msg.Format("array:  %7ld ticks\nparray: %7ld ticks\ncarray: %7ld ticks\nvector: %7ld ticks\n",
  t2-t1, t4-t3, t6-t5, t8-t7);

  AfxMessageBox(msg);
============
Typical output is

array:   1469 ticks
parray: 1344 ticks
carray: 2750 ticks
vector: 1359 ticks

There are two really interesting things here:

1) dynamically created array and vector are consistently 8-10% faster than static array. I have no idea why.

2) CArray is a dog. Stepping into the operator[] code, I eventually find this:

template<class TYPE, class ARG_TYPE>
AFX_INLINE TYPE& CArray<TYPE, ARG_TYPE>::ElementAt(INT_PTR nIndex)
{
      ASSERT(nIndex >= 0 && nIndex < m_nSize);
      if(nIndex >= 0 && nIndex < m_nSize)
            return m_pData[nIndex];
      AfxThrowInvalidArgException();            
}

CArray is doing bounds checking even in release mode! This is why it is so slow.

So unless you need bounds checking and are willing to pay a 100% penalty for it, I wouldn't use CArray.

Just for kicks I ran the above code in debug mode:

array:   4656 ticks
parray: 3844 ticks
carray: 19000 ticks
vector: 63734 ticks (!!!)

Holy moly, I wonder why vectors are so bad in debug mode.

It looks like you should get your old code back from storage - better performance and should not really be a problem re memory.

wayside, that was an impressive way to demonstrate the point. It seems that I am really better off with arrays, dynamically sized. My problem was that the code ran "out of memeory" too often, even without reaching the ststic limit. I thought CArray was the answer. Now I know better.

ran "out of memeory" too often

from what you have said of the array contents and sizes that is a surprising comment.  Where you assigning but not freeing memory?

>  My problem was that the code ran "out of memeory" too often, even without reaching the ststic limit

Well, this is a completely different problem. Can you describe the conditions where it happens? How much physical memory do you have? How big is your swap file?

ok, just give me sometime to make an inventory of the arrays I have. there are quite a number of them.

I have a follow-up question:

I used dynamic array as suggested before by wayside:

int *myarray;
// get how big we need
howbig = 100;
myarray = new int[howbig];

and clean it up on exit:

delete [] myarray;

Now, this array is a member of an Object OO, which in turn is a member of another Object SuperOO. Where am I supposed to delete the array when I don't use it anymore? I use an initialize function and a delete function for object OO (not the default destructor), but the deletion is not working and I get a crash. What am I doing wrong?

> What am I doing wrong?

Impossible to say unless you post some code. I think we need to see at least your initializaton and delete functions, and an example of how you are using them.

I'm not sure why you don't want to use a destructor to free up this memory. Have the delete function set the pointer to NULL, and have the destructor check and free it if necessary:

void OO::delete() {
  if (myArray != NULL) {
     delete [] myArray;
     myArray = NULL;
  }
}

OO::~OO {
  if (myArray != NULL) {
     delete [] myArray;
     muArray = NULL;
  }
}

Ok here is a code fragment, simplified:

//In Header for OO:
int*      array;
===================
//In Header for SuperOO:
extern OO** OOArray;
===================


SuperOO::SuperOO(Size1) {

OOArray = new OO[Size1];
// work with it
// then delete it
if(OOArray!=NULL) delete OOArray; // probably wrong!
}
====================

OO::InitOO(int Size2) {
array = new int[Size2];
}

OO::~OO() {
delete [] array;
}

========================================

The current situation is:
delete [] array causes a crash!

You need to have a constructor for OO so you can initialize array:

OO::OO() : array(NULL) {}

then in the destructor, test before you delete:

OO::~OO() {
  if (array)
     delete [] array;
}


> extern OO** OOArray;

I think this should be

extern OO* OOArray; // one dimensional array

and this line:

> if(OOArray!=NULL) delete OOArray; // probably wrong!

should be

if(OOArray!=NULL) delete [] OOArray;

When you new with [] you must also delete with [].

Thanks for your patience!

extern OO** OOArray;
This is because OOArray is actually an array of pointers.

What does this statement actually do?
OO::OO() : array(NULL) {}

I initialise array in a special functionOO::InitOO();
Do I HAVE to use the constructor?

Everything seems to be working fine, except when I try to delete array with [], I get the Debug Error message:
DAMAGE: after Normal block (#3365) at 0x00AC8CF0.

>This is because OOArray is actually an array of pointers.

But you are allocating it like this:

OOArray = new OO[Size1];

which is not giving you an array of pointers, it's giving you an array of objects. The confusion here is probably because you are not giving the real code but are trying to construct a small example.


> What does this statement actually do?
> OO::OO() : array(NULL) {}

This implements a default contructor and use special syntax to initialize the array member to NULL. In this case it is equivalent to

OO:OO() {
  array = NULL;
}

but there are times when you when to have to use the special member initialization syntax.

If array isn't null, and you create and delete an OO object without calling the Init() function, you will be deleting memory you never allocated.

> Do I HAVE to use the constructor?

If you don't specifiy one, the compiler will most likely create a default constuctor for you. So there is zero penalty to have one.

> DAMAGE: after Normal block (#3365) at 0x00AC8CF0

This most likely means you have written past the end of the allocated space. In debug mode, memory allocators typically allocate more than you ask for, and put special "guard" byte values at the beginning and and. Then, when you delete the memory, it can check if the guard bytes have changed. If they have, it means you've written beyond the bounds of your memory.

Thanks for the hint on the crash. It was indeed reaching outside the allocated space.

> OO::OO() : array(NULL) {}
1. Is this initialization necessary?
2. Is there a way to initialize all the array elements?

> > OO::OO() : array(NULL) {}
> 1. Is this initialization necessary?

It's not strictly necessary if you can guarantee that OO::InitOO() always gets called before the object is destroyed. But it is good programming practice to initialize all your variables.

> 2. Is there a way to initialize all the array elements?

If the array elements are objects, the constructor will get called for each array element, you can do any initialization there. If the array is a basic type, you could initialize them in the Init))() function:

OO::InitOO(int Size2) {
  array = new int[Size2];

  for (int i=0; i<Size2; i++)
    array[i] = 0;
}

Ok, thanks for the valuable help. Other than having to revisit my constructors and destructors again, things seem to be running fine now.

Is there a way in VisualC++ to discover and report memory leaks?

Ok, I will see what I can do. thanks again for the great help