1
\$\begingroup\$

I want to compare the procedure time for each number of process. I made 5000000 numbers in input.txt in random order. I read it and do bucket sorting with multiprocessing (let the number of buckets = number of process) and I record the time spending to do bucket sorting. I am wondering if my code is doing multiprocessing well and is it right to take about 1.xx seconds to do it per each number of process? I understand that the hardware performance is the important thing but I think it may be too short.

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <time.h>

int N = 5000000; // 숫자의 개수
int numProcessesOptions[] = {1, 3, 5, 7, 10, 20, 40, 70, 100, 150, 200, 250, 300};
int numProcessesCount = sizeof(numProcessesOptions) / sizeof(numProcessesOptions[0]);

// 비교 함수를 정의합니다.
int compareDoubles(const void *a, const void *b) {
    return (*(double *)a - *(double *)b > 0) - (*(double *)a - *(double *)b < 0);
}

void bucketSort(double *arr, int n, int processID, int numProcesses) {
    // 데이터를 분할합니다.
    int chunkSize = N / numProcesses;
    int startIndex = processID * chunkSize;
    int endIndex = (processID == numProcesses - 1) ? N : (processID + 1) * chunkSize;

    // 버킷 소트를 수행합니다.
    int *buckets = (int *)calloc(chunkSize, sizeof(int));
    if (buckets == NULL) {
        perror("메모리 할당 실패");
        exit(1);
    }

    for (int i = startIndex; i < endIndex; i++) {
        int index = (int)((arr[i] - 1.0) * chunkSize);
        buckets[index]++;
    }

    int index = startIndex;
    for (int i = 0; i < chunkSize; i++) {
        while (buckets[i] > 0) {
            arr[index++] = (double)i / chunkSize + 1.0;
            buckets[i]--;
        }
    }

    free(buckets);
}
void writeToFile(double *arr, int n, char *filename) {
    FILE *outputFile = fopen(filename, "w");

    if (outputFile == NULL) {
        perror("파일을 열 수 없습니다.");
        return;
    }

    for (int i = 0; i < n; i++) {
        fprintf(outputFile, "%lf\n", arr[i]); // 소수점 10자리까지 출력
    }

    fclose(outputFile);
}

void mergeData(double *arr, int n, int numProcesses) {
    double *mergedData = (double *)malloc(N * sizeof(double));
    if (mergedData == NULL) {
        perror("메모리 할당 실패");
        exit(1);
    }

    int chunkSize = N / numProcesses;
    int process = 0;

    for (int i = 0; i < N; i++) {
        if (i % chunkSize == 0 && process < numProcesses) {
            process++;
        }
        mergedData[i] = arr[i];
    }

    // 모든 프로세스에서 생성된 데이터를 병합하여 정렬합니다.
    qsort(mergedData, N, sizeof(double), compareDoubles);

    // 정렬된 데이터를 파일에 저장합니다.
    writeToFile(mergedData, N, "output.txt");

    free(mergedData);
}


void sortAndMeasureTime(int numProcesses) {
    double *numbers = (double *)malloc(N * sizeof(double));
    if (numbers == NULL) {
        perror("메모리 할당 실패");
        return;
    }

    for (int i = 0; i < N; i++) {
        // 데이터 초기화 (1.0부터 5000000까지)
        numbers[i] = i + 1.0;
    }

    clock_t start_time = clock();

    // 각 프로세스에서 데이터를 버킷 소트합니다.
    for (int i = 0; i < numProcesses; i++) {
        pid_t pid = fork();

        if (pid == 0) {
            // 자식 프로세스에서 버킷 소트 수행
            bucketSort(numbers, N, i, numProcesses);
            exit(0);
        } else if (pid < 0) {
            perror("프로세스 생성 실패");
            exit(1);
        }
    }

    // 모든 자식 프로세스의 종료를 기다립니다.
    for (int i = 0; i < numProcesses; i++) {
        int status;
        wait(&status);
    }

    // 모든 프로세스에서 생성된 데이터를 병합하여 정렬합니다.
    mergeData(numbers, N, numProcesses);

    clock_t end_time = clock();
    double total_time = (double)(end_time - start_time) / CLOCKS_PER_SEC;

    free(numbers); // 메모리 해제

    printf("Process: %d, Time: %.5f seconds\n", numProcesses, total_time);
}

int main() {
    for (int i = 0; i < numProcessesCount; i++) {
        sortAndMeasureTime(numProcessesOptions[i]);
    }

    return 0;
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Enable more compiler warnings:

gcc -std=c99 -fPIC -gdwarf-4 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wconversion  -Wstrict-prototypes -fanalyzer bb.c -o bb

bb.c: In function ‘bucketSort’:

bb.c:24:34: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]

   24 |     int *buckets = (int *)calloc(chunkSize, sizeof(int));

      |                                  ^~~~~~~~~

bb.c:17:34: warning: unused parameter ‘n’ [-Wunused-parameter]

   17 | void bucketSort(double *arr, int n, int processID, int numProcesses) {

      |                              ~~~~^

bb.c: In function ‘mergeData’:

bb.c:61:45: warning: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result [-Wsign-conversion]

   61 |     double *mergedData = (double *)malloc(N * sizeof(double));

      |                                             ^

bb.c:78:23: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]

   78 |     qsort(mergedData, N, sizeof(double), compareDoubles);

      |                       ^

bb.c:81:32: warning: passing argument 3 of ‘writeToFile’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

   81 |     writeToFile(mergedData, N, "output.txt");

      |                                ^~~~~~~~~~~~

bb.c:45:44: note: expected ‘char *’ but argument is of type ‘const char *’

   45 | void writeToFile(double *arr, int n, char *filename) {

      |                                      ~~~~~~^~~~~~~~

bb.c:60:33: warning: unused parameter ‘n’ [-Wunused-parameter]

   60 | void mergeData(double *arr, int n, int numProcesses) {

      |                             ~~~~^

bb.c: In function ‘sortAndMeasureTime’:

bb.c:88:42: warning: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result [-Wsign-conversion]

   88 |     double *numbers = (double *)malloc(N * sizeof(double));

      |                                          ^

bb.c: At top level:

bb.c:132:5: warning: function declaration isn’t a prototype [-Wstrict-prototypes]

  132 | int main() {

      |     ^~~~

Declare internal functions static:

As we are not exporting anything from this translation unit, all functions except main() should be defined with internal linkage.

Simplify the comparison function:

It is more traditionally written as:

#if 0
int compareDoubles(const void *a, const void *b) {
    return (*(double *)a - *(double *)b > 0) - (*(double *)a - *(double *)b < 0);
}
#else 
int compareDoubles(const void *a, const void *b) {
    const double *const da = a;
    const double *const db = b;
    
    return (*da > *db) - (*da < *db);
}
#endif

Or simply:

int compareDoubles(const void *a, const void *b) {
    return (*(const double *) a > *(const double *) b)
           - (*(const double *) a < *(const double *) b);
}

Reduce redundant casts:

There's no need to cast the return value of malloc() and family. The returned void * is implicitly converted to any other pointer type.

fopen() is not required to set errno:

Consider:

#if 0
    FILE *outputFile = fopen(filename, "w");
    if (outputFile == NULL) {
        perror("파일을 열 수 없습니다.");
        return;
    }
#else 
    FILE *outputFile = (errno = 0, fopen(filename, "w"));

    if (!outputFile) {
        errno ? perror("파일을 열 수 없습니다.") : (void) fputs( /* Error message here. */, stderr);
        return;
    }
#endif

Use standard status code:

stdlib.h provides EXIT_FAILURE and EXIT_SUCCESS. Prefer using them to 0, 1, et cetera.

Don't use exit() in the child branch of a fork():

It is normally incorrect to use exit() in the child branch of a fork(), because that can lead to stdio buffers being flushed twice, and temporary files being unexpectedly removed. Use _exit().

Use const for data that is not modified:

It also better documents the code.

#if 0
void writeToFile(double *arr, int n, char *filename) {
    FILE *outputFile = fopen(filename, "w");
...
#else
static void writeToFile(const double *arr, int n, const char *filename) {
    FILE *const outputFile = fopen(filename, "w");
...
#endif

Eliminate unused variables:

The parameter n in mergeData() goes unused.

Return an error code to the caller instead of exiting the program:

mergeData() should not call exit() in case of a malloc() failure. Use the return value and let the caller decide what to do.

Use size_t for sizes, cardinalities, or ordinal numbers:


// int N = 5000000; // 숫자의 개수
size_t N = 5000000;
int numProcessesOptions[] = {1, 3, 5, 7, 10, 20, 40, 70, 100, 150, 200, 250, 300};
// int numProcessesCount = sizeof(numProcessesOptions) / sizeof(numProcessesOptions[0]);
size_t  numProcessesCount = sizeof(numProcessesOptions) / sizeof(numProcessesOptions[0]);

Use unsigned for small quantities that can't be negative:

// int numProcessesOptions[] = {1, 3, 5, 7, 10, 20, 40, 70, 100, 150, 200, 250, 300};
unsigned int numProcessesOptions[] = {1, 3, 5, 7, 10, 20, 40, 70, 100, 150, 200, 250, 300};

See: What are the valid signatures for C's main() function?

\$\endgroup\$
2
\$\begingroup\$

bucketSort() causes a segmentation fault

Your calculation of index in bucketSort() is wrong, and causes the process to read outside of the array buckets[], which causes the process to crash with a segmentation fault. You don't see this because you are not catching this error because you ignore the value of status after calling wait().

So the time you see is just the time spent in mergeData(). Since that is running in a single process, it will be the same every time.

Processes don't share memory

When you call fork(), you create a child process which has a copy of the memory of the parent process. What the child process does with this memory can however not be seen by the parent. So while bucketSort() sorts the bucket in its process's memory, mergeData() will not see the results. You are merging the unsorted data from the parent process instead.

There are various ways to make this work:

  • Write the sorted buckets to files, and have mergeData() read those files in.
  • Use sockets or pipes to communicate between the processes and send the sorted data from the child processes to the parent.
  • Create a shared memory region using System V or POSIX shared memory functions.
  • Use threads instead of processes, since threads do share their memory.

mergeData() does a full sort

Your mergeData() just calls qsort() on the result of the bucket sort. Why bucket sort at all then? The whole point of sorting the buckets is that you can use a much more efficient merge algorithm on the result.

Use as many threads or processes as your CPU has cores

Using more threads or processes than you have CPU cores is not useful for an algorithm like this. The overhead of managing many processes will just slow down the sorting at some point. So at the very least, limit it to the number of CPU cores that are available. On a CPU with many cores that might still be too much, as at some point you will be limited by the available memory bandwidth.

\$\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.