help finding an efficient way to copy dynamic memory!!

L

laclac01

I have developed my own copy function for coping my own dynamic memory
structure. It works, but I feel its not too efficient. There must be
a quicker way to copy the data. In some of the routines I have
developed the copy function gets called several hundreds of times, with
very large data structures. This can take some time....

If you can't tell by the code the data structure is a matrix for
storing complex numbers.

Attached is the code.

struct complexMatrix
{
float r;
float i;
};

typedef complexMatrix** ComplexMatrix;
typedef complexMatrix Complex;

ComplexMatrix Cmxcopy(ComplexMatrix mat, int rows, int cols)
{

int i;
int j;
ComplexMatrix temp;
temp=CmxAlloc(rows,cols);

for(i=0;i<rows;i++)
for(j=0;j<cols;j++)
{
temp[j].r=mat[0][0].r;
temp[j].i=mat[0][0].i;
}

return temp;

}


All suggesting, comments, emails, faxes, post-its and memos welcome.
 
F

Frank Chang

laclac1, There is a potential for memory leakage after executing
CmxCopy unless you implement the correct destructor for ComplexMatrix.
 
V

Victor Bazarov

I have developed my own copy function for coping my own dynamic memory
structure. It works, but I feel its not too efficient. There must be
a quicker way to copy the data. In some of the routines I have
developed the copy function gets called several hundreds of times, with
very large data structures. This can take some time....

If you can't tell by the code the data structure is a matrix for
storing complex numbers.

Attached is the code.

struct complexMatrix
{
float r;
float i;
};

typedef complexMatrix** ComplexMatrix;
typedef complexMatrix Complex;

ComplexMatrix Cmxcopy(ComplexMatrix mat, int rows, int cols)
{

int i;
int j;
ComplexMatrix temp;
temp=CmxAlloc(rows,cols);

This is very C-like. Why don't you start writing C++? Only declare
objects when they are needed, and avoid defining without initialising:

ComplexMatrix temp = CmxAlloc(rows, cols);
for(i=0;i<rows;i++)

for (int i=0; i<rows; i++)
for(j=0;j<cols;j++)

for (int j=0; j<cols; j++)
{
temp[j].r=mat[0][0].r;
temp[j].i=mat[0][0].i;


Why are you copying the top left element of the 'mat'? Shouldn't this be

temp[j].r=mat[j].r;
temp[j].i=mat[j].i;
???

Since your 'complexMatrix' struct is a POD you _may_ use memcpy to copy
objects of that type. That might improve things a bit:

memcpy(&temp[j], &mat[j], sizeof(temp[j]));

And it can be improved even further if you keep the pointer to the current
element around and increment it every inner loop, instead of indexing (for
some compilers it gives a bit of advantage).

Another way to actually improve it is to avoid dual allocation/filling
of the matrix and instead develop a 'allocate_and_copy' and use it in
place of 'CmxAlloc' and the loops.
return temp;

}


All suggesting, comments, emails, faxes, post-its and memos welcome.

V
 
V

Victor Bazarov

Frank said:
laclac1, There is a potential for memory leakage after executing
CmxCopy unless you implement the correct destructor for ComplexMatrix.

Frank, please pay attention. 'ComplexMatrix' is a pointer to a pointer
to 'complexMatrix' struct. There is no destructor.
 
F

Frank Chang

Victor Bazarov, Yes, I know ComplexMatrix is a pointer to a pointer.
Suppose this scenario occurs,

int main()
{
int rows(1000);
int cols(1000);

ComplexMatrix s = CmxAlloc(rows, cols)
ComplexMatrix b = CmxCopy(s,rows,cols);
CmxFree(s); // deallocates memory for s

return 0;
}

Now ,if the user fails also to deallocate the memory for ComplexMatrix
b, which is a pointer to a pointer, there will be a memory leak. The
reason I used the term destructor is because 'ComplexMatrix' should
really be a C++ template class as I pointed out last week.
 
A

annamalai.gurusami

I have developed my own copy function for coping my own dynamic memory
structure. It works, but I feel its not too efficient. There must be
a quicker way to copy the data. In some of the routines I have
developed the copy function gets called several hundreds of times, with
very large data structures. This can take some time....

Having one contiguous block of memory representing one matrix would
help in creating copies faster. (This is my assumption. Hope that
is valid.)

How about something like this? In my example, I am using two blocks
of memory, one holding the actual data (or the complex numbers) and
the other holding the meta data. Any comments on this are welcome.

It is C code, run through C++ compiler. Converting this into a
suitable C++ class is left as an exercise.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct ComplexNumber
{
float r;
float i;
};

struct Matrix
{
int maxRow;
int maxCol;
size_t size;
ComplexNumber *first;
};

Matrix* matrix_alloc(int row, int col)
{
Matrix* matrix = (Matrix*) malloc( sizeof(Matrix) );
matrix->maxRow = row;
matrix->maxCol = col;
matrix->size = sizeof(ComplexNumber)*row*col;
matrix->first = (ComplexNumber*) malloc( matrix->size );
return matrix;
}

ComplexNumber* matrix_get(Matrix* matrix, int row, int col)
{
int idx = (row* matrix->maxCol)+col;
return matrix->first+idx;
}

void matrix_free(Matrix* x)
{
free(x->first);
free(x);
}
Matrix* matrix_clone(Matrix* orig)
{
Matrix* copy = matrix_alloc(orig->maxRow, orig->maxCol);
memcpy(copy->first, orig->first, orig->size);
return copy;
}

void matrix_print(Matrix *matrix)
{
printf("\n");
for(int i=0; i < matrix->maxRow; ++i)
{
for(int j=0; j< matrix->maxCol; ++j)
{
ComplexNumber* cn = matrix_get(matrix, i,j);
printf("(%f, %f) ", cn->r, cn->i);
}
printf("\n");
}
printf("\n");
}

int main()
{
Matrix *matrix = matrix_alloc(3, 3);

for(int i=0; i <3; i++)
{
for(int j=0; j<3; j++)
{
ComplexNumber* cn = matrix_get(matrix, i,j);
cn->r = i;
cn->i = j;
}
}
Matrix *copy = matrix_clone(matrix);
matrix_print(matrix);
matrix_print(copy);
matrix_free(matrix);
matrix_free(copy);
return 0;
}
 
L

laclac01

How about only having one memcopy per matrix copy...?
I thought this would work but i guess it doesn't
memcpy(&temp, &m, rows*cols*2*sizeof(float));
Is there a way to make this work? I am looking in to the last
suggestion.
 
A

annamalai.gurusami

How about only having one memcopy per matrix copy...?

But the code I posted has only one memcpy per matrix copy.

Is it just me or does anyone else also think that, the operation
is more appropriately called clone rather than a copy?

Rgds,
anna
 
L

laclac01

Yes your code does, but i would like to not have to redo all of my
existing code.... Is there anyway to optmize with my code to do this?
clone or copy works for me :)
 
V

Victor Bazarov

[...]
Is it just me or does anyone else also think that, the operation
is more appropriately called clone rather than a copy?

How would you define the difference?
 
A

Alf P. Steinbach

* Victor Bazarov:
[...]
Is it just me or does anyone else also think that, the operation
is more appropriately called clone rather than a copy?

How would you define the difference?

That a pure copy operation copies into existing (provided) storage?
 
A

annamalai.gurusami

Yes your code does, but i would like to not have to redo all of my
existing code.... Is there anyway to optmize with my code to do this?
clone or copy works for me :)

Your code is here

ComplexMatrix Cmxcopy(ComplexMatrix mat, int rows, int cols)
{

int i;
int j;
ComplexMatrix temp;
temp=CmxAlloc(rows,cols);

for(i=0;i<rows;i++)
for(j=0;j<cols;j++)
{
temp[j].r=mat[0][0].r;
temp[j].i=mat[0][0].i;
}

return temp;

}

And if you are not willing to avoid those two for loops, I am
wondering what is left to optimize in this? Converting those
two float assignments into a memcpy is not going to make that
much of a difference. Since you will introduce one more
function call (or can it be inlined?) it can actually reduce
performance.

Rgds,
anna
 
A

annamalai.gurusami

Alf said:
* Victor Bazarov:
[...]
Is it just me or does anyone else also think that, the operation
is more appropriately called clone rather than a copy?

How would you define the difference?

That a pure copy operation copies into existing (provided) storage?

Thats what I had in mind. The C standard functions like strcpy,
memcpy, is done this way only.

Rgds,
anna
 
S

sap.liu

I think that you just should copy the whole block of memory of the
matrix, like below:

struct complexMatrix
{
float r;
float i;
};

typedef complexMatrix** ComplexMatrix;
typedef complexMatrix Complex;

ComplexMatrix CmxCopy(ComplexMatrix mat, int rows, int cols)
{
ComplexMatrix temp;
temp=CmxAlloc(rows,cols);
::memcpy(temp, mat, rows * cols * sizeof(complexMatrix));
return temp;
}

and just like what Frank said, you should remember to realloc the
memory allocated in the function, after you don't use it.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,898
Latest member
BlairH7607

Latest Threads

Top