3
\$\begingroup\$


A code to check the time slice for each thread separately, it will receive an integer number from the command line for the number of threads to run simultaneously. Would love some feedback on what is possible to be done differently in order to improve it (e.g different functions to use which are more accurate?, etc.).
Thank you.

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

int curThread = 0;
void* f(void* thread_serial);

void main(int argc, char* argv[]){
    int N;//amount of threads to activate
    int i;//will be used as an index and thread's serial number
    int status;//will be used to indicate a failed thread creation
    int* serials;//will be used to save the serial number of a thread
    pthread_t* threads;

    //check if the amount of parameters is valid 
    if (argc < 2){
        printf("ERROR - missing parameters\n");
        exit(EXIT_FAILURE);
    }


    N = atoi(argv[1]);//saves the amount of threads provided by the user

    threads = (pthread_t*)malloc(N * sizeof(pthread_t));//allocate threads
    serials = (int*)malloc(N * sizeof(int));//allocate array for the serial number of threads

    //check if allocation failed
    if (threads == NULL || serials == NULL){
        printf("ERROR - memory allocation failed!");
        exit(EXIT_FAILURE);
    }

    for (i = 0; i < N; i++){
        serials[i] = i + 1;

        //create new thread
        status = pthread_create(&threads[i], NULL, f, (void*)&serials[i]);

        //check if thread allocation failed
        if (status != 0){
            printf("ERROR - failed to create a thread");
            exit(EXIT_FAILURE);
        }
    }

    sleep(2);
    free(threads);
    free(serials);
}

void* f(void* thread_serial){
    int k = *(int*)thread_serial;
    struct timeval thread1, thread2;
    double elapsedTime;

    while(1){
        gettimeofday(&thread1, NULL);//start the timer
        curThread = k;//get the thread serial
        
        //while on the same thread
        while (curThread == k)
            gettimeofday(&thread2, NULL); //update the timer
        
        //compute and print the elapsed time in millisec
        elapsedTime = (thread2.tv_sec - thread1.tv_sec) * 1000.0;//sec to ms
        elapsedTime += (thread2.tv_usec - thread1.tv_usec) / 1000.0;//us to ms
        printf("Time slice for thread %d = %lf ms.\n", curThread, elapsedTime);
    }
}

One possible output:

Time slice for thread 1 = 54.410000 ms.
Time slice for thread 2 = 10.530000 ms.
Time slice for thread 3 = 44.491000 ms.
Time slice for thread 5 = 66.553000 ms.
Time slice for thread 4 = 56.311000 ms.
Time slice for thread 1 = 44.143000 ms.
Time slice for thread 3 = 43.631000 ms.
Time slice for thread 5 = 78.483000 ms.
Time slice for thread 2 = 56.536000 ms.
Time slice for thread 4 = 55.217000 ms.
Time slice for thread 1 = 56.470000 ms.
Time slice for thread 3 = 56.644000 ms.
Time slice for thread 5 = 43.937000 ms.
Time slice for thread 4 = 45.256000 ms.

The program finds the approximate time needed to for each thread (separately) to finish it's run. For example, if 5 threads are created, then the thread will take the start time (in f function) and while that same thread is running it will get another time (for finish), and print the finish minus start time of that same thread run, when another thread starts running, it's ID (serial) will be different, thus making those calculations for this "new" thread.

Since the loop is an infinite loop, the program will keep changing between the threads and print each of their run time (until the program finishes it's run).

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

This does not work

There are several issues with this code. It may partially stem from a slight misunderstanding of how threads work:

Since the loop is an infinite loop, the program will keep changing between the threads and print each of their run time (until the program finishes it's run).

It doesn't matter if there is a loop or not or whether it is infinite or not. Furthermore, it's not the program that "keeps changing between threads"; it is the operating system that will assign threads to CPU cores, and if there are more threads than cores, decide how and when to give time slices to threads.

Your program assumes that only one thread can ever run at the same time. Nowadays, all desktop and server CPUs that are sold have multiple cores, and even microcontrollers like the ESP32 and RP2040 have multiple cores. If you have multiple cores, then your code is already wrong to begin with.

However, it is worse: even if you only have a single core that is running all your threads, then there still is no guarantee when a thread will be stopped and another will be started. Consider these lines:

while (curThread == k)
    gettimeofday(&thread2, NULL); //update the timer

A thread running this code might get interrupted right between the check for the condition curThread == k and the call to gettimeofday(). But this means this thread will think it was running for much longer than it actually did.

In fact, there is no way to measure the size of time slices assigned to threads from within that thread. At best you can repeatedly call gettimeofday() and check how long it was after the previous call to gettimeofday(). If that happened within the same timeslice, the difference in time will be very small. If it's suddenly much larger, then that might be an indication that it was interrupted by another thread. However, there are many reasons why this is inaccurate:

  • CPU clock speed matters, and clock speeds can vary while the program is running.
  • There are other things besides threads that can interrupt your program for various amounts of time, like the operating system's housekeeping interrupts, system management interrupts, small or large jumps in the clock due to NTP updates, and so on.

Print error messages to stderr

It's great to see that you check the return value from most functions, and handle errors accordingly! However, make sure you print errors to stderr instead of stdout; this ensures it doesn't mix with the normal, expected output from your program, and more importantly, will ensure the error messages will still be visible if stdout is redirected to a file for example.

It might also help to print strerror(errno), so the user has more information about the cause of the error. Note that there are library functions to help with this, for example C's perror(), or if you only target Linux and/or BSD based operating systems, err() is even more convenient.

main() must return int

The function main() must always return an int for it to be standards compliant. Note however that you don't need an explicit return statement in the body of main().

Incorrect #include

The function gettimeofday() requires that you #include <sys/time.h>, not <time.h>.

Possibility to avoid memory allocation

You are calling malloc() twice, and it's great to see you clean up with free(). However, you could have avoided both allocations. First, in C you can create a dynamically sized array on the stack:

int N = atoi(argv[1]);
pthread_t threads[N];
int serials[N];

However, even those two arrays are not needed. First, since you don't actually care about the thread handles, you don't have to store them at all. Second, you can convert an integer to a pointer and back, which will allow you to pass the serial number without needing an allocation. To do this without compiler warnings, ensure you convert it to intptr_t inbetween:

for (i = 0; i < N; i++) {
    pthread_t thread;
    int status = pthread_create(&thread, NULL, f, (void *)(intptr_t)(i + 1));
    if (status != 0) {
        err(EXIT_FAILURE, "Failed to create a thread");
    }
}
...
void* f(void* thread_serial) {
    int k = (intptr_t)thread_serial;
    ...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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