Thread Sincronization PRoblem (Mandlebrot)

[Hyp3rTension]

Hello i am trying to create mandlebrot images using threads ... i have managed to create them using only one process now i want to create them using threads... i have managed to do this .. but i have not managed to synchronize the threads. I need some help in making each new thread wait for the thread before to put something in the buffer before running.

Here is the code:

#include "recursos.h"
double *b;
pthread_mutex_t mut;
int max_iterations = 256;


/* This function (compute_point) is the algorithmic core of this program,
   computing what could be called the Mandelbrot function of the complex
   number (cr, ci).
*/

double compute_point(double ci, double cr) {
  int iterations = 0;
  double zi = 0;
  double zr = 0;
  while ((zr*zr + zi*zi < 4) && (iterations < max_iterations)) {
    double nr, ni; 

    /* Z <-- Z^2 + C */
   
    nr = zr*zr - zi*zi + cr;
    ni = 2*zr*zi + ci;
    
    zi = ni;
    zr = nr;

    iterations ++;
  }  
  return iterations;  
}

/* The "compute" function computes the Mandelbrot function over every
   point on a grid that is "nx" points wide by "ny" points tall, where
   (xmin,ymin) and (xmax,ymax) give two corners of the region the 
   complex plane.
*/

void compute(double *buffer, int nx, int ny, double xmin, double xmax, 
         double ymin, double ymax) {
  double delta_x, delta_y;
  int x, y;

  delta_x = (xmax - xmin)/nx;
  delta_y = (ymax - ymin)/ny;

  for (y=0;  y<ny; y++) {
    double y_value = ymin + delta_y * y;
    for (x=0; x<nx; x++) {
      double x_value = xmin + delta_x * x;
      buffer[y*nx + x] = compute_point(x_value, y_value);
    }
  }
}

/* Output the data contained in the buffer to a Portable Greymap format
   image file.  The parameter "max" should be an upper bound for the
   data values in the buffer. 
*/

void output_pgm(char *filename,double *buffer, int nx, int ny, double max) {
  int i;
  FILE *file;
  file = fopen(filename,"w");
  fprintf(file,"P2\n");
  fprintf(file,"%d %d\n",nx,ny);
  fprintf(file,"%d\n",(int)max);
  for (i=0; i<nx*ny; i++) {
    if (!(i%nx)) fprintf(file,"\n");
    fprintf(file,"%d ",(int)buffer[i]);
  }
  fclose(file);
}
 
void *computeFunc(void *args) {
   //  printf("TestChegar\n");
     struct threadArgs *targs;
     targs = (struct threadArgs *)args;
     int n = targs->nFatias;
     double yM = targs->yMin;
     printf("%f\n",yM);
     double multiplier = ((1.0/n) * 2);
     pthread_mutex_lock(&mut);
     compute(b,1000,(int)1000/n,-1,1,yM,(yM+multiplier));
     char str[80]; 
     char c[10]; 
     sprintf(c,"%d",(targs->indice+1)); 
     strcpy(str,"mandel-"); 
     strcat(str,c); 
     strcat(str,".pgm"); 
     output_pgm(str,b,1000,(1000/n),255);
     b = b + (1000/n)*1000;
     pthread_mutex_unlock(&mut); 
}

int main(int argc, char *argv[]) {
    if(argc != 2) {
    
    printf("Introduziu o formato errado para o programa \n O formato correcto é : ./nomeprog <nrworkers>\n");

    } else {    

    pthread_mutex_init(&mut,NULL);
    int nWorkers = atoi(argv[1]);        
    int i = 0;
    pthread_t threads[nWorkers];

    struct threadArgs targs[nWorkers];
    
    double *ptr;
    b= ptr = malloc(1000*1000*sizeof(double));

    double aux = -1.0;
    double multiplier = ((1.0/nWorkers) * 2);
    for(i = 0; i < nWorkers;i++) {
         targs[i].nFatias = nWorkers;
        targs[i].yMin = aux;
        targs[i].b = b;
        targs[i].indice = i;    
        pthread_create(&threads[i],NULL,computeFunc,(void *)&targs[i]);
        aux = aux + multiplier;
    }
    pthread_join(threads[nWorkers - 1],NULL);
    output_pgm( "mandel.pgm", ptr, 1000, 1000, 255);

    }

    
}

And the header file:

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

struct threadArgs{
   int nFatias;
   double yMin;
   int indice;
   double *b;
};

[oogabooga]

What's the point of the mutexes here? Don't you want these threads all working at once?

[Salem]

> b= ptr = malloc(1000*1000*sizeof(double));
Perhaps you should allocate separate memory for each thread, for the image size they work on.

Because at the moment, all the threads fight over the SAME block of global memory.

If you allocate what you need for each thread, AND make this bit of code use the targs->b member (and NOT the global), then you might get somewhere.

     compute(b,1000,(int)1000/n,-1,1,yM,(yM+multiplier));
     char str[80];
     char c[10];
     sprintf(c,"%d",(targs->indice+1));
     strcpy(str,"mandel-");
     strcat(str,c);
     strcat(str,".pgm");
     output_pgm(str,b,1000,(1000/n),255);
     b = b + (1000/n)*1000;  //!! WTF!!!

Delete the global variable, then compile and fix what it wrong.

[Hyp3rTension]

Oh yes .. i understand ..
the problem is that .. i would need that global variable because the main picture would have to contain the whole drawing
is there no way of synchronizing the threads so that all of them finish what they have to do then wait for the thread before to write to the buffer before writing themselves
the mutex are just to ensure mutual exclusion , i don't know if they can be ommited.

[Salem]

So do you want one global "picture" and each thread works on a part of the picture?

Then after ALL the threads have passed join, you output the image from main?
pthread_join(threads[nWorkers - 1],NULL); //!! join ALL of them
output_pgm( "mandel.pgm", ptr, 1000, 1000, 255);

Sure, that would work, but you need to make sure each thread ONLY operates on the part of the image it's supposed to.

[Hyp3rTension]

ohhh thank you .. you helped me get to the solution ..
it's exactly what you said ..
I have to join each thread..
and yes each thread works on a part of the image and then we output evertything on the main ...
the outputs on the threadfunction are just tests ...


Thank you!

[Hyp3rTension]

Here is the Working code

#include "recursos.h"
double *b;
pthread_mutex_t mut;
int max_iterations = 256;


/* This function (compute_point) is the algorithmic core of this program,
   computing what could be called the Mandelbrot function of the complex
   number (cr, ci).
*/

double compute_point(double ci, double cr) {
  int iterations = 0;
  double zi = 0;
  double zr = 0;
  while ((zr*zr + zi*zi < 4) && (iterations < max_iterations)) {
    double nr, ni; 

    /* Z <-- Z^2 + C */
   
    nr = zr*zr - zi*zi + cr;
    ni = 2*zr*zi + ci;
    
    zi = ni;
    zr = nr;

    iterations ++;
  }  
  return iterations;  
}

/* The "compute" function computes the Mandelbrot function over every
   point on a grid that is "nx" points wide by "ny" points tall, where
   (xmin,ymin) and (xmax,ymax) give two corners of the region the 
   complex plane.
*/

void compute(double *buffer, int nx, int ny, double xmin, double xmax, 
         double ymin, double ymax) {
  double delta_x, delta_y;
  int x, y;

  delta_x = (xmax - xmin)/nx;
  delta_y = (ymax - ymin)/ny;

  for (y=0;  y<ny; y++) {
    double y_value = ymin + delta_y * y;
    for (x=0; x<nx; x++) {
      double x_value = xmin + delta_x * x;
      buffer[y*nx + x] = compute_point(x_value, y_value);
    }
  }
}

/* Output the data contained in the buffer to a Portable Greymap format
   image file.  The parameter "max" should be an upper bound for the
   data values in the buffer. 
*/

void output_pgm(char *filename,double *buffer, int nx, int ny, double max) {
  int i;
  FILE *file;
  file = fopen(filename,"w");
  fprintf(file,"P2\n");
  fprintf(file,"%d %d\n",nx,ny);
  fprintf(file,"%d\n",(int)max);
  for (i=0; i<nx*ny; i++) {
    if (!(i%nx)) fprintf(file,"\n");
    fprintf(file,"%d ",(int)buffer[i]);
  }
  fclose(file);
}
 
void *computeFunc(void *args) {
   //  printf("TestChegar\n");
     struct threadArgs *targs;
     targs = (struct threadArgs *)args;
     int n = targs->nFatias;
     double yM = targs->yMin;
     printf("%f\n",yM);
     double multiplier = ((1.0/n) * 2);
     pthread_mutex_lock(&mut);
     compute(b,1000,(int)1000/n,-1,1,yM,(yM+multiplier));
 /*  
     char str[80]; 
     char c[10]; 
     sprintf(c,"%d",(targs->indice+1)); 
     strcpy(str,"mandel-"); 
     strcat(str,c); 
     strcat(str,".pgm"); 
     output_pgm(str,b,1000,(1000/n),255);
*/
     b = b + (1000/n)*1000;
     pthread_mutex_unlock(&mut); 
}

int main(int argc, char *argv[]) {
    if(argc != 2) {
    
    printf("Introduziu o formato errado para o programa \n O formato correcto é : ./nomeprog <nrworkers>\n");

    } else {    

    pthread_mutex_init(&mut,NULL);
    int nWorkers = atoi(argv[1]);        
    int i = 0;
    pthread_t threads[nWorkers];

    struct threadArgs targs[nWorkers];
    
    double *ptr;
    b= ptr = malloc(1000*1000*sizeof(double));

    double aux = -1.0;
    double multiplier = ((1.0/nWorkers) * 2);
    for(i = 0; i < nWorkers;i++) {
         targs[i].nFatias = nWorkers;
        targs[i].yMin = aux;
        targs[i].b = b;
        targs[i].indice = i;    
        pthread_create(&threads[i],NULL,computeFunc,(void *)&targs[i]);
        aux = aux + multiplier;
        pthread_join(threads[i],NULL);
    }
    output_pgm( "mandel.pgm", ptr, 1000, 1000, 255);

    }

    
}

[Salem]

You're still running threads serially with that mutex in place.

Plus you're still messing with a global variable.

Each thread should have just ONE pointer to one bit of exclusive memory, and then you can run the threads in parallel.

[oogabooga]

You're still running threads serially with that mutex in place.

And even if you removed them from the above code, the way you use pthread_join ALSO defeats the whole purpose of the exercise. Move it out of that loop into a loop of its own. And be sure to inline compute_point to avoid the call overhead of one million calls.