Slow pthread consumer

Question

I've implemented a solution the producer / consumer problem in C using pthreads and semaphores.

My main thread is the producer and I launch N consumer threads.

My code is:

typedef struct
{
    int buf[BUFSIZE];     /* shared var */
    int in;               /* buf[in%BUFSIZE] is the first empty slot */
    int out;              /* buf[out%BUFSIZE] is the first full slot */
    sem_t full;           /* keep track of the number of full spots */
    sem_t empty;          /* keep track of the number of empty spots */
    pthread_mutex_t mutex;          /* enforce mutual exclusion to shared data */
} CONSUMER_STRUCT;

CONSUMER_STRUCT shared;

This is the code for each of my consumer threads:

void *Consumer(void *arg)
{
    int fd, workerID, i, hit=0;

    workerID = *(int *)arg;

    for (;;) {
        sem_wait(&shared.full);
        pthread_mutex_lock(&shared.mutex);
        fd = shared.buf[shared.out];
        printf("\n[C%d] Consumed. I got  %d ...Valor do buffer: %d na posição %d\n\n\n", workerID, fd, shared.buf[shared.out], shared.out);
        ftp(fd, hit);
        shared.buf[shared.out] = 0;
        shared.out = (shared.out+1)%BUFSIZE;
        fflush(stdout);
        printf("\n\n\n\nEstado do buffer:\n\n\n\n");
        for (i = 0; i < BUFSIZE; i++) {
            //printf("%d ", shared.buf[i]);
        }
        /* Release the buffer */
        pthread_mutex_unlock(&shared.mutex);
        /* Increment the number of full slots */
        sem_post(&shared.empty);
        hit++;
    }
    return NULL;
}

And this is the code for my producer thread:

item = socketfd;

sem_wait(&shared.empty);
pthread_mutex_lock(&shared.mutex);

shared.buf[shared.in] = item;

shared.in = (shared.in + 1) % BUFSIZE;
fflush(stdout);

pthread_mutex_unlock(&shared.mutex);
sem_post(&shared.full);

Everything is working properly but serving 22 files takes around 20 seconds whilst creating one thread per request takes around 2 seconds! This seems to be executing a thread at a time and I want to execute all of them "at the same time".

Am I doing something wrong in my implementation approach?

Your consumers are holding the mutex during effectively their entire operation. You're running single-threaded with synchronization overhead. — EOF, Apr 19 '16 at 16:01
I understood what you meant! What should be the correct approach to this problem? — rafaelcpalmeida, Apr 19 '16 at 16:03
Let's assume for a moment that `ftp()` in `Consumer()` dominates the consumer's execution time. Further, let's assume `ftp()` is thread-safe. Then, you can just move `ftp()` below the `pthread_mutex_unlock()` and actually execute concurrently. — EOF, Apr 19 '16 at 16:06
Wait on the sema, lock, pop queue, unlock. Don't do anything inside the lock that you don't absolutely have to. The longer you hold the lock, the greater the chance of contention. Same with producer/s - what is that 'fflush(stdout);' inside the lock for? Lock, push, unlock, signal. — Martin James, Apr 19 '16 at 16:07
Not sure what 'shared.buf[shared.out] = 0;' is for. Some debug aid? — Martin James, Apr 19 '16 at 16:14
Thanks for your tips guys, it's working correctly now. I thought I needed to use `shared.buf[shared.out] = 0;` to zero the buffer in order to work correctly. — rafaelcpalmeida, Apr 19 '16 at 23:40

score 0 · Accepted Answer · answered Apr 19 '16 at 23:44

For those who might come here with a similar problem, here's the fix.

Thanks to @Martin James and @EOF.

void *Consumer(void *arg)
{
    int fd, workerID, i, hit=0;

    workerID = *(int *)arg;

    for (;;) {
        sem_wait(&shared.full);
        pthread_mutex_lock(&shared.mutex);
        fd = shared.buf[shared.out];
        shared.buf[shared.out] = 0;
        shared.out = (shared.out+1)%BUFSIZE;
        pthread_mutex_unlock(&shared.mutex);
        printf("\n[C%d] Consumed. I got  %d ...Valor do buffer: %d na posição %d\n\n\n", workerID, fd, shared.buf[shared.out], shared.out);
        ftp(fd, hit);
        fflush(stdout);
        printf("\n\n\n\nEstado do buffer:\n\n\n\n");
        for (i = 0; i < BUFSIZE; i++) {
            //printf("%d ", shared.buf[i]);
        }
        /* Release the buffer */
        /* Increment the number of full slots */
        sem_post(&shared.empty);
        hit++;
    }
    return NULL;
}

The problem is that I was locking the mutex, executing a function and then unlocking the mutex. This is what was causing so much delay on the execution.

Slow pthread consumer

1 Answers1