1

I tried to make a parallel program that generates a random number with one thread and the other thread writes it.

Am I doing something wrong that messes with the performance/optimization? I ask it because it was very easy to write this program so I'm a little concerned that I am doing something wrong.

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <pthread.h>
#include <semaphore.h>
#include <time.h>
#include "produceConsume.h"
#define NUM_THREAD 1

pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;

int queue[1];
int queueCounter = 0;

void *producer(void *args)
{
    while (1)
    {
        pthread_mutex_lock(&lock);
        int n = rand() % 100;
        queue[queueCounter] = n;
        queueCounter++;
        pthread_cond_wait(&cond, &lock);
        pthread_mutex_unlock(&lock);
    }
}

void *consumer(void *args)
{
    while (1)
    {
        pthread_mutex_lock(&lock);
        printf("%d\n", queue[queueCounter - 1]);
        queueCounter--;
        pthread_cond_signal(&cond);
        pthread_mutex_unlock(&lock);
        sleep(1);
    }
}

int main()
{
    system("clear");
    srand(time(NULL));
    pthread_t th[NUM_THREAD], th2[NUM_THREAD];

    for (int i = 0; i < NUM_THREAD; i++)
    {
        pthread_create(&th[i], NULL, &producer, NULL);
        pthread_create(&th2[i], NULL, &consumer, NULL);
    }

    for (int i = 0; i < NUM_THREAD; i++)
    {
        pthread_join(th[i], NULL);
        pthread_join(th2[i], NULL);
    }
}
  • As the code is written, increasing `NUM_THREAD` would be senseless. Also you don't `return` from the threads so the code has undefined behavior bugs all over (like your compiler told you). – Lundin Nov 03 '21 at 12:59
  • I use wsl linux so I didn't saw the warnings –  Nov 03 '21 at 13:01
  • Take a look at the CPU load this produces. The producer will spin wildly while not doing all that much. In a good implementation the CPU should remain mostly idle. And the usual pattern for a producer/consumer is to have a FIFO queue, what you have there behaves like a LIFO stack. (you won't notice the difference for random numbers, try increasing numbers instead...) –  Nov 03 '21 at 13:01
  • @Lundin I don't increase NUM_THREAD –  Nov 03 '21 at 13:02
  • @dratenik I looked it up and you're right it produces a lot of CPU load. How can I fix this? Thank you for helping! –  Nov 03 '21 at 13:06
  • You can use a pthread_cond variable to signalize between threads. When the queue is empty, the consumer should sleep and wait for new data, the produces signalizes when new data arrives. When the queue is full the producer should sleep and the consumer signalizes when space is freed up. So for a maximum implementation you need two condition vars. You can test them by speeding up/slowing down producer/consumer with varying sleeps. Some explanation of cond vars: https://stackoverflow.com/questions/16522858/understanding-of-pthread-cond-wait-and-pthread-cond-signal –  Nov 03 '21 at 13:16
  • @dratenik Thank you I'm gonna write it again and post it here –  Nov 03 '21 at 13:19
  • @dratenik Can you check the code now, I changed it. –  Nov 03 '21 at 14:23

1 Answers1

0

You don't need an array if you are going to use only one thread, in any case, you create two threads but only one is joined (leaking memory), instead:

pthread_t th1[NUM_THREAD]; // or simply pthread_t th1;
pthread_t th2[NUM_THREAD]; // or simply pthread_t th2;

for (int i = 0; i < NUM_THREAD; i++)
{
    pthread_create(&th1[i], NULL, &producer, NULL);
    pthread_create(&th2[i], NULL, &consumer, NULL);
}

for (int i = 0; i < NUM_THREAD; i++)
{
    pthread_join(th1[i], NULL);
    pthread_join(th2[i], NULL);
}
David Ranieri
  • 39,972
  • 7
  • 52
  • 94
  • Stupid mistake of me, thnx –  Nov 03 '21 at 13:20
  • I changed the code can you check it? –  Nov 03 '21 at 14:23
  • Much better now without the waste of cpu. Two minor things: `srand(time(NULL));` -> `srand((unsigned)time(NULL));` and `system(clear);` is a pain if you are looking for performance, since you are on unix you can make use of console codes to clear the screen: `printf("\033[H\033[2J");` – David Ranieri Nov 03 '21 at 14:36