Параллельное векторное умножение с использованием нескольких потоков занимает больше времени, чем последовательное

Question

У меня есть две функции, которые делают умножение двух векторов целых чисел (на данный момент заполнены всеми). Я ожидаю, что функция vector_multiplication_concurrent, которая использует потоки, будет быстрее, чем функция vector_multiplication. Тем не менее, это на самом деле немного медленнее. Я подозреваю, что это потому, что только один поток одновременно работает с переменной result, поэтому потоки фактически не выполняют работу параллельно. Это правильно? Как мне изменить код, чтобы параллельная функция была быстрее?

Код:

#include <iostream>
#include <chrono>
#include <vector>
#include <thread>
#include <mutex>

void vector_multiplication(std::vector<int> const & v1,
                           std::vector<int> const & v2,
                           int & result) {

    for (int ind = 0; ind < v1.size(); ++ind) {
        result += v1[ind] * v2[ind];
    }

}

static std::mutex mtx;
void vector_multiplication_concurrent(std::vector<int> const & v1,
                                     std::vector<int> const & v2,
                                     int start_ind, int end_ind,
                                     int & result) {


    std::lock_guard<std::mutex> lck(mtx);

    for (int ind = start_ind; ind <= end_ind; ++ind) {
        result += v1[ind] * v2[ind];
    }

}

int main(){

    std::vector<int> v1 (10000000, 1);
    std::vector<int> v2 (10000000, 1);

    int result = 0;

    std::chrono::high_resolution_clock::time_point t1 = std::chrono::high_resolution_clock::now();
    vector_multiplication(v1, v2, result);
    std::chrono::high_resolution_clock::time_point t2 = std::chrono::high_resolution_clock::now();

    auto duration = std::chrono::duration_cast<std::chrono::microseconds>(t2 - t1).count();
    std::cout << "Duration: " << duration << '\n';
    std::cout << "Product: " << result << '\n';


    int result_concurrent = 0;
    int threads_num = 4;
    std::vector<std::thread> threads;

    std::chrono::high_resolution_clock::time_point t3 = std::chrono::high_resolution_clock::now();

    for (int th = 0; th < threads_num; ++th) {
        threads.push_back(std::thread(vector_multiplication_concurrent,
                                      std::ref(v1),
                                      std::ref(v2),
                                      th * (v1.size() / threads_num),
                                      th * (v1.size() / threads_num) + v1.size() / threads_num - 1,
                                      std::ref(result_concurrent)));
    }
    for (auto & th : threads) {
        th.join();
    }

    std::chrono::high_resolution_clock::time_point t4 = std::chrono::high_resolution_clock::now();

    auto duration_concurrent = std::chrono::duration_cast<std::chrono::microseconds>(t4 - t3).count();
    std::cout << "Duration concurrent: " << duration_concurrent << '\n';
    std::cout << "Product concurrent: " << result_concurrent << '\n';


    return 0;
}

Answer 1

Как уже упоминалось в комментариях, вы блокируете мьютекс на всю продолжительность функции, поэтому код на самом деле является последовательным. Вам нужен мьютекс, только если несколько потоков обращаются к одной и той же памяти и хотя бы один пишет.

В случае суммирования векторных элементов вам нужно иметь только несколько потоков, записывающих в одну и ту же память при добавлении окончательного результата, поэтому вы можете изменить функцию на:

static std::mutex mtx;
void vector_multiplication_concurrent(std::vector<int> const & v1,
                                     std::vector<int> const & v2,
                                     int start_ind, int end_ind,
                                     int & result) {

    // fully parallel part
    // v1 and v2 are shared, but you are only reading
    int temp = 0;

    for (int ind = start_ind; ind <= end_ind; ++ind) {
        temp += v1[ind] * v2[ind];
    }
    // only this requires you to synchronize access 
    // result is shared and you are writing to it
    std::lock_guard<std::mutex> lck(mtx);
    result += temp;
}

PS: я бы настоятельно рекомендовал вам использовать итераторы вместо индексов. Также обратите внимание, что ваш цикл в основном переписан на std::inner_product. Использование этого вместо простого цикла сделает ваш код более выразительным.