Incomplete multi-threading RayTracer taking twice as much time as expected

Question

I am making a MT Ray-Tracer multithreading, and as the title says, its taking twice as much to execute as the single thread version. Obviously the purpose is to cut the render time by the half, however what I am doing now is just to send the ray-tracing method to run twice, one for each thread, basically executing the same rendering twice. Nonetheless, as threads can run in parallel, shall not there be a meaningful increase in execution time. But is about doubling.

This has to be related to my multithreading setup. I think its related to the fact I create them as joinable. So I am going to explain what I am doing and also put the related code to see if someone can confirm if that's the issue.

I create two threads and set them as joinable so. Create a RayTracer that allocates enough memory to store the image pixels (this is done in the constructor). Run a two iterations loop for sending relevant info for each thread, like the thread id and the adress of the Raytracer instance.

Then pthread_create calls run_thread, whose purpose is to call the ray_tracer:draw method where the work is done. On the draw method, I have a

 pthread_exit (NULL); 

as the last thing on it (the only MT thing on it). Then do another loop to join the threads. Finally I star to write the file in a small loop. Finally close the file and delete the pointers related to the array used to store the image in the draw method.

I may not need to use to join now that I am not doing a "real" multithreading ray-tracer, just rendering it twice, but as soon as I start alternate between the image pixels (ie, thread0 -> renders pixel0 - thread0 -> stores pixel0, thread1 -> renders pixel1 - thread1 -> stores pixel1, thread0 -> renders pixel2 - thread0 -> stores pixel2, , thread1 -> renders pixel3 - thread1 -> stores pixel3,etc...) I think I will need it so to be able to write the pixels in correct order on a file.

Is that correct? Do I really need to use join here with my method (or with any other?). If I do, how can I send the threads to run concurrently, not waiting for the other to complete? Is the problem totally unrelated to join?

pthread_t threads [2];
thread_data td_array [2];
pthread_attr_t attr;
void *status;
pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
TGAManager tgaManager ("z.tga",true);
if (tgaManager.isFileOpen()) {
   tgaManager.writeHeadersData (image);
   RayTracer rt (image.getHeight() * image.getWidth());
   int rc;
   for (int i=0; i<2; i++) {
      //cout << "main() : creating thread, " << i << endl;
      td_array[i].thread_id=i;
      td_array[i].rt_ptr = &rt;
      td_array[i].img_ptr = &image;
      td_array[i].scene_ptr = &scene;
      //cout << "td_array.thread_index: " << td_array[i].thread_id << endl;
      rc = pthread_create (&threads[i], NULL, RayTracer::run_thread, &td_array[i]);
  }
   if (rc) {
      cout << "Error:unable to create thread," << rc << endl;
      exit(-1);
   }
   pthread_attr_destroy(&attr);
   for (int i=0; i<2; i++ ) {
      rc = pthread_join(threads[i], &status);
      if (rc) {
         cout << "Error:unable to join," << rc << endl;
         exit(-1);
      }
   }
//tgaManager.writeImage (rt,image.getSize());

  for (int i=0; i<image.getWidth() * image.getHeight(); i++) {
    cout << i << endl;
    tgaManager.file_writer.put (rt.b[i]);
    tgaManager.file_writer.put (rt.c[i]);
    tgaManager.file_writer.put (rt.d[i]);
  }
tgaManager.closeFile(1);
rt.deleteImgPtr (); 
}

Answer 1

You do want to join() the threads, because if you don't, you have several problems:

1. How do you know when the threads have finished executing? You don't want to start writing out the resulting image only to find that it wasn't fully calculated at the moment you wrote it out.

2. How do you know when it is safe to tear down any data structures that the threads might be accessing? For example, your RayTracer object is on the stack, and (AFAICT) your threads are writing into its pixel-array. If your main function returns before the the threads have exited, there is a very good chance that the threads will sometimes end up writing into a RayTracer object that no longer exists, which will corrupt the stack by overwriting whatever other objects might exist (by happenstance) at those same locations after your function returned.

So you definitely need to join() your threads; you don't need to explicitly declare them as PTHREAD_CREATE_JOINABLE, though, since that attribute is already set by default anyway.

Joining the threads should not cause the threads to slow down, as long as both threads are created and running before you call join() on any of them (which appears to be the case in your posted code).

As for why you are seeing a slowdown with two threads, that's hard to say since a slowdown could be coming from a number of places. Some possibilities:

1. Something in your ray-tracing code is locking a mutex, such that for much of the ray-tracing run, only one of the two threads is allowed to execute at a time anyway.

2. Both threads are writing to the same memory locations at around the same time, and that is causing cache-contention which slows down the execution of both threads.

My suggestion would be to set your threads so that thread #1 renders only the top half of the image, and thread #2 renders only the bottom half of the image; that way when they write their output they will be writing to different sections of memory.

If that doesn't help, you might temporarily replace the rendering code with something simpler (e.g. a "renderer" that just sets pixels to random values) to see if you can see a speedup with that. If so, then there might be something in your RayTracer's implementation that isn't multithreading-friendly.