4
\$\begingroup\$

I have a working program that reads user input keys and echoes them back to the screen using the producer/consumer paradigm (the project requires you to use threads).

While this program does work, it is unfortunately very inefficient. When I run the 'top' command, the CPU usage is over 200. Any suggestions to modify this code so it's not using up a significant percentage of the CPU? This is my first program using threads so I'm not sure if I am doing something wrong if this is normal haha. Thanks in advance.

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

#define NITEMS 10       // number of items in shared buffer

// shared variables
char shared_buffer[NITEMS]; // echo buffer
int shared_count;       // item count

pthread_mutex_t mutex;      // pthread mutex
unsigned int prod_index = 0;    // producer index into shared buffer
unsigned int cons_index = 0;    // consumer index into shard buffer

// function prototypes
void * producer(void *arg);
void * consumer(void *arg);

int main() 
{ 
    pthread_t prod_tid, cons_tid1, cons_tid2; 

    // initialize pthread variables
    pthread_mutex_init(&mutex, NULL);
    
    // start producer thread
    pthread_create(&prod_tid, NULL, producer, NULL);

    // start consumer threads
    pthread_create(&cons_tid1, NULL, consumer, NULL);
    pthread_create(&cons_tid2, NULL, consumer, NULL);
    
    // wait for threads to finish
    pthread_join(prod_tid, NULL);
    pthread_join(cons_tid1, NULL);
    pthread_join(cons_tid2, NULL);
            
    // clean up
    pthread_mutex_destroy(&mutex);
    
    return 0;
}

// producer thread executes this function
void * producer(void *arg)
{
    char key;

    printf("Enter text for producer to read and consumer to print, use Ctrl-C to exit.\n\n");

    // this loop has the producer read in from stdin and place on the shared buffer
    while (1)
    {
        // read input key
        scanf("%c", &key);

        // this loop is used to poll the shared buffer to see if it is full:
        // -- if full, unlock and loop again to keep polling
        // -- if not full, keep locked and proceed to place character on shared buffer
        while (1)
        {
            // acquire mutex lock
            pthread_mutex_lock(&mutex);

            // if buffer is full, release mutex lock and check again
            if (shared_count == NITEMS)
                pthread_mutex_unlock(&mutex);
            else
                break;
        }

        // store key in shared buffer
        shared_buffer[prod_index] = key;

        // update shared count variable
        shared_count++;

        // update producer index
        if (prod_index == NITEMS - 1)
            prod_index = 0;
        else
            prod_index++;
        
        // release mutex lock
        pthread_mutex_unlock(&mutex); 
    }

    return NULL;
}

// consumer thread executes this function
void * consumer(void *arg)
{
    char key;

    long unsigned int id = (long unsigned int)pthread_self();

    // this loop has the consumer gets from the shared buffer and prints to stdout
    while (1)
    {
        // this loop is used to poll the shared buffer to see if it is empty:
        // -- if empty, unlock and loop again to keep polling
        // -- if not empty, keep locked and proceed to get character from shared buffer
        while (1)
        {
            // acquire mutex lock
            pthread_mutex_lock(&mutex);

            // if buffer is empty, release mutex lock and check again
            if (shared_count == 0)
                pthread_mutex_unlock(&mutex);
            else
                break;
        }

        // read key from shared buffer
        key = shared_buffer[cons_index];
        
        // echo key
        printf("consumer %lu: %c\n", (long unsigned int) id, key);

        // update shared count variable
        shared_count--;

        // update consumer index
        if (cons_index == NITEMS - 1)
            cons_index = 0;
        else
            cons_index++;
    
        // release mutex lock
        pthread_mutex_unlock(&mutex);
    }

    return NULL;
}
\$\endgroup\$
1
  • \$\begingroup\$ Over 200 what? Threads? Percent? Common sense dictates that it isn't possible to use more than 100% of a CPU. \$\endgroup\$
    – Lundin
    Oct 29 '20 at 7:21
3
\$\begingroup\$

Input is s l o w. Most of the time the buffer is empty, and the producer patiently waits for scanf to return. So the consumer loop

    while (1)
    {
        // acquire mutex lock
        pthread_mutex_lock(&mutex);

        // if buffer is empty, release mutex lock and check again
        if (shared_count == 0)
            pthread_mutex_unlock(&mutex);
        else
            break;
    }

breaks very rarely. Most of the time it just busy waits. That explains the high CPU usage.

Producer-consumer scenario requires not one, but two blocking points, one to block the producer, another to block the consumer. An easiest way out is to have two counting semaphores, free_space (initialized to NITEMS), and data_space (initialized to 0). Producer shall wait on free_space and signal data_space, and consumer shall do vice versa.

\$\endgroup\$
1
\$\begingroup\$

Pay attention when things explode

Let's do some reading.

If successful, the pthread_mutex_init() and pthread_mutex_destroy() functions return zero. Otherwise, an error number is returned to indicate the error. The [EBUSY] and [EINVAL] error checks, if implemented, act as if they were performed immediately at the beginning of processing for the function and cause an error return prior to modifying the state of the mutex specified by mutex.

This matters. For all we know, every single one of your pthread calls could be failing and you wouldn't know it. C doesn't hold your hand through this sort of thing - no exceptions are thrown; it's your responsibility to read the documentation for every function you call and check every return value for failure.

\$\endgroup\$
4
  • \$\begingroup\$ would that effect the cpu at all? \$\endgroup\$
    – user232489
    Oct 29 '20 at 1:48
  • \$\begingroup\$ Yep. If a function inside a loop is failing and returning immediately, and the loop spins forever... how much CPU would that eat up? \$\endgroup\$
    – Reinderien
    Oct 29 '20 at 2:06
  • \$\begingroup\$ ah okay I see, I was reading about conditional variables. I haven't used them before, would that be useful in this case? \$\endgroup\$
    – user232489
    Oct 29 '20 at 2:09
  • 1
    \$\begingroup\$ @user232489 Yes, condition variables would help. But also search on CodeReview for posts from other people who submitted C++ implementations of multi-threaded producer/consumer queues, and study their code and the answers given there. \$\endgroup\$
    – G. Sliepen
    Oct 29 '20 at 17:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.