help finding an efficient way to copy dynamic memory!!

Discussion in 'C++' started by laclac01@yahoo.com, Aug 24, 2005.

  1. Guest

    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.
    , Aug 24, 2005
    #1
    1. Advertising

  2. Frank Chang Guest

    laclac1, There is a potential for memory leakage after executing
    CmxCopy unless you implement the correct destructor for ComplexMatrix.
    Frank Chang, Aug 24, 2005
    #2
    1. Advertising

  3. wrote:
    > 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
    Victor Bazarov, Aug 24, 2005
    #3
  4. Frank Chang wrote:
    > 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.
    Victor Bazarov, Aug 24, 2005
    #4
  5. Frank Chang Guest

    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.
    Frank Chang, Aug 24, 2005
    #5
  6. Guest

    wrote:

    > 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;
    }
    , Aug 25, 2005
    #6
  7. Guest

    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.
    , Aug 25, 2005
    #7
  8. Guest

    wrote:
    > 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
    , Aug 25, 2005
    #8
  9. Guest

    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 :)
    , Aug 25, 2005
    #9
  10. wrote:
    > [...]
    > 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?
    Victor Bazarov, Aug 25, 2005
    #10
  11. * Victor Bazarov:
    > wrote:
    > > [...]
    > > 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: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Aug 25, 2005
    #11
  12. Guest

    the diffrence? with what?
    , Aug 25, 2005
    #12
  13. * :
    > the diffrence? with what?


    Please teach yourself common rules for quoting by checking typical postings
    in this group.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Aug 25, 2005
    #13
  14. Guest

    I am not sure i get whats going on.
    , Aug 25, 2005
    #14
  15. Guest

    wrote:
    > 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
    , Aug 26, 2005
    #15
  16. Guest

    Alf P. Steinbach wrote:
    > * Victor Bazarov:
    > > wrote:
    > > > [...]
    > > > 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
    , Aug 26, 2005
    #16
  17. sap.liu Guest

    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.
    sap.liu, Aug 27, 2005
    #17
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Alex
    Replies:
    2
    Views:
    1,224
  2. Guest
    Replies:
    4
    Views:
    571
    Steven Cheng[MSFT]
    Aug 27, 2007
  3. Developer
    Replies:
    4
    Views:
    711
    Daniel Pitts
    Feb 25, 2009
  4. Replies:
    3
    Views:
    104
    Anno Siegel
    Jan 6, 2005
  5. John Reye
    Replies:
    43
    Views:
    1,330
    Malcolm McLean
    Nov 14, 2013
Loading...

Share This Page