• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 332
  • Last Modified:

Performance problems!!!

I have two dynamic arrays:

int* poli1 = NULL ;
int* poli2 = NULL ;

I need a function that receive by reference poli1 and poli2, calculate the new size of poli1 and poli2, reallocate poli1 and poli2, fill with zero the (new_size - oldsize) first positions of the new array and mantain the old values till the end of new array.

I was trying something like this:

void prepareArrays ( int* poli1 , int* poli2, int oldSize )
{
    int new_size = calcSize (poli1, poli2) ;

    int *new_poli1 = ( int * )calloc( (new_n) , sizeof( int ) ) ;
    int *new_poli2 = ( int * )calloc( (new_n) , sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
      {
            new_poli1[ i + new_size - n ] = p[i] ;
            new_poli2[ i + new_n - n ] = q[i] ;
      }

    poli1 = new_poli1 ;
    poli2 = new_poli2 ;
}

Two problems:
1) Too Slow and memory expensive
2) When out the function prepareArrays()  poli1 array is not with values of new_poli1 array

Any sugestions ??
Please help!






0
Gonella
Asked:
Gonella
  • 6
  • 2
  • 2
  • +1
3 Solutions
 
PaulCaswellCommented:
Hi Gonella,

I'm assuming calcSize is not the issue.

The trick is to allocate a much larger array each time and record its length within a structure. When it comes to concatenate the arrays, you check to see if there is already enough room, just use more of one of the buffers.

This strategy is difficult to do from scratch. You'd be better to find an enhanced memory allocation module to handle it for you.

I'd also recommend investigating malloc, memcpy and memset rather than using calloc. calloc will clear ALL of the memory. You dont need that.

Use memcpy instead of the 'i' loop too.

Paul
0
 
Kent OlsenData Warehouse Architect / DBACommented:
Hi Gonella,

The function has a number of problems, most of which can be resolved.

You're passing two integer pointers but don't use them.  Just before the function ends, it sets the passed pointers to the start of the arrays that you assign.  Here's the kicker -- you're putting the first integer of the new arrays into the first position of the old arrays.  You are not returning the address of these buffers.

Also, you're using p[] and q[] for new values.  Are these global variables or do you mean to use the passed arrays?

What's new_n?  I don't see it defined here either.


Kent
0
 
GonellaAuthor Commented:
With this posted code, why can't change poli1 values ? When this function ends, it losts the values of poli1 and poli2.

I trying to make this simple sample works. I tryied:

void prepareArrays ( int* poli1 , int* poli2, int oldSize )
{
    int new_size = calcSize (poli1, poli2) ;

    int *new_poli1 = ( int * )calloc( (new_n) , sizeof( int ) ) ;
    int *new_poli2 = ( int * )calloc( (new_n) , sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
     {
          new_poli1[ i + new_size - n ] = p[i] ;
          new_poli2[ i + new_n - n ] = q[i] ;
     }

    poli1 = (int*) realloc ( poli1, new_n * sizeof(int) ) ;
    poli2 = (int*) realloc ( poli2, new_n * sizeof(int) ) ;
            
    poli1 = (int*) memcpy ( poli1, new_p, new_n * sizeof(int) );
    poli2 = (int*) memcpy ( poli2, new_q, new_n * sizeof(int) );
}

But its not working.
Why I'm lossing values of poli1 and poli2 outside this function ??
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
GonellaAuthor Commented:
Sorry:

new_n = new_size

copy and paste problems!
0
 
GonellaAuthor Commented:
Due several copy and paste errors:
REPOSTING:

void prepareArrays ( int* poli1 , int* poli2, int oldSize )
{
    int new_size = calcSize (oldSize) ;   // Returns the log2(oldSize)

    int *new_poli1 = ( int * )calloc( (new_size) , sizeof( int ) ) ;
    int *new_poli2 = ( int * )calloc( (new_size) , sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
     {
          new_poli1[ i + new_size - oldSize ] = poli1[i] ;
          new_poli2[ i + new_size - oldSize ] = poli2[i] ;
     }

    poli1 = (int*) realloc ( poli1, new_size * sizeof(int) ) ;
    poli2 = (int*) realloc ( poli2, new_size * sizeof(int) ) ;
         
    poli1 = (int*) memcpy ( poli1, new_poli1, new_n * sizeof(int) );
    poli2 = (int*) memcpy ( poli2, new_poli2, new_n * sizeof(int) );
}

Outside this function i need to use poli1 and poli2, but not working...
0
 
PaulCaswellCommented:
This should work:

void prepareArrays ( int** poli1 , int** poli2, int oldSize )
{
    int new_size = calcSize (oldSize) ;   // Returns the log2(oldSize)

    int *new_poli1 = ( int * )calloc( (new_size) , sizeof( int ) ) ;
    int *new_poli2 = ( int * )calloc( (new_size) , sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
     {
          new_poli1[ i + new_size - oldSize ] = *poli1[i] ;
          new_poli2[ i + new_size - oldSize ] = *poli2[i] ;
     }

    *poli1 = new_poli1; //(int*) realloc ( poli1, new_size * sizeof(int) ) ;
    *poli2 = new_poli2; //(int*) realloc ( poli2, new_size * sizeof(int) ) ;
         
    //poli1 = (int*) memcpy ( poli1, new_poli1, new_n * sizeof(int) );
    //poli2 = (int*) memcpy ( poli2, new_poli2, new_n * sizeof(int) );
}

And call it with:

int * pol1 = malloc (...);
int * pol2 = malloc (...);

prepareArrays ( &poli1 , &poli2, oldSize );

Also note that you will leak the memory from the old arrays. You need to free them.

Paul
0
 
GonellaAuthor Commented:
Well, it's not working!
Here is the implemented code based on yours.

void prepareArrays ( int** poli1 , int** poli2, int oldSize )
{
    int new_size = calcSize (oldSize) ;  

    int *new_poli1 = ( int * )calloc( (new_size) , sizeof( int ) ) ;
    int *new_poli2 = ( int * )calloc( (new_size) , sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
     {
          new_poli1[ i + new_size - oldSize ] =  poli1[0][i]              //*poli1[i] ;       // have to change this line
          new_poli2[ i + new_size - oldSize ] =  poli2[0][i]              //*poli2[i] ;        // have to change this line
     }

    *poli1 = new_poli1;
    *poli2 = new_poli2;
   
    free(new_poli1);          // lost all values of poli1     :(
    free(new_poli2);          // lost all values of poli2     :(

}

Please help!!
0
 
GonellaAuthor Commented:
Kdo, please look at the last code posted...

poli1 and poli2 are global variables defined in main.cpp while prepareArrays () function is defined in mp.cpp file.

0
 
brettmjohnsonCommented:
>   *poli1 = new_poli1;
>    *poli2 = new_poli2;
>   
>    free(new_poli1);          // lost all values of poli1     :(
>    free(new_poli2);          // lost all values of poli2     :(


You are freeing the memory that you are returning to the caller.
The caller will subsequently read or write freed memory, with unpredictable results,
since that freed memory becomes available for future allocations.

0
 
GonellaAuthor Commented:
Here it is: now working

void prepareArrays ( int** poli1 , int** poli2, int oldSize )
{
    int new_size = calcSize (oldSize) ;  

    int *new_poli1 = ( int * )malloc( (new_size) * sizeof( int ) ) ;
    int *new_poli2 = ( int * )malloc( (new_size) * sizeof( int ) ) ;

    for( int i = 0 ; i < n ; i++ )
     {
          new_poli1[ i + new_size - oldSize ] =  poli1[0][i]            
          new_poli2[ i + new_size - oldSize ] =  poli2[0][i]              
     }

     for( int i = 0 ; i < new_size - oldSize ; i++ )
     {
           new_poli1[i] = 0 ;
           new_poli2[i] = 0 ;
     }
            
     *poli1 = (int*)realloc(*poli1, new_size * sizeof(int) );
     *poli2 = (int*)realloc(*poli2, new_size * sizeof(int) );

     *poli1 = (int*)memcpy(*poli1, new_poli1, new_size * sizeof(int) );
     *poli2 = (int*)memcpy(*poli2, new_poli2, new_size * sizeof(int) );
             
    free(new_poli1);          
    free(new_poli2);          

}

Any ideas to make it faster??

The main idea is:

poli1 = { 1,2,3,4,5 }
poli2 = { 2,3,5,6,7 }

if new_size = 8, then...

poli1 = { 0,0,0,1,2,3,4,5 }
poli2 = { 0,0,0,2,3,5,6,7 }

think in terms of millions...

0
 
Kent OlsenData Warehouse Architect / DBACommented:

You can make it quite a bit faster by not moving items one at a time.

In your example, you know that the new array will be 3 integers larger than the old and that the first 3 integers in the new array will be zero.  That makes things pretty easy.

  new_poli1 = (int *)  malloc (new_size * sizeof(int));                                  // Allocate the new buffer
  memset (new_poli1, 0, (new_size - old_size) * sizeof (int));                       // Set the leading integers to zero.
  memcpy (&new_poli1[new_size - old_size], poli1, old_size * sizeof (int));   //  Move the old array to the new one
  free (poli1);

Then repeat those same 4 lines for poli2.  :)


Kent
0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

  • 6
  • 2
  • 2
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now