4
\$\begingroup\$

I wrote a C++ class which can take a vector of functions and their respective arguments and execute them in parallel and return a vector of results from its respective functions.

// @file: mthread.hh
// @author: Tushar Chaurasia (Dark-CodeX)
// @license: This file is licensed under the GNU GENERAL PUBLIC LICENSE Version 3, 29 June 2007. You may obtain a copy of this license at https://www.gnu.org/licenses/gpl-3.0.en.html.

#ifndef MTHREAD_HH
#define MTHREAD_HH

#include <vector>
#include <thread>
#include <functional>

namespace openutils
{
    template <typename return_type, typename... args>
    class mthread
    {
    private:
        std::vector<return_type> ret_val;
        std::vector<std::thread> threads;

    public:
        void execute_processes(const std::vector<std::function<return_type(args...)>> &functions, const std::vector<args> &...a);
        const std::vector<return_type> &get() const;
        std::vector<return_type> &get();
    };

    template <typename return_type, typename... args>
    void mthread<return_type, args...>::execute_processes(const std::vector<std::function<return_type(args...)>> &functions, const std::vector<args> &...a)
    {
        std::vector<return_type> temp_val(functions.size());        // pre allocate
        this->threads = std::vector<std::thread>(functions.size()); // pre allocate

        for (std::size_t i = 0; i < functions.size(); i++)
        {
            this->threads[i] = std::thread([i, &temp_val, &functions, &a...]()
                                           { temp_val[i] = (functions[i](a[i]...)); });
        }

        for (std::size_t i = 0; i < this->threads.size(); i++)
        {
            this->threads[i].join();
        }

        this->ret_val.swap(temp_val);
    }

    template <typename return_type, typename... args>
    const std::vector<return_type> &mthread<return_type, args...>::get() const
    {
        return this->ret_val;
    }

    template <typename return_type, typename... args>
    std::vector<return_type> &mthread<return_type, args...>::get()
    {
        return this->ret_val;
    }
}

#endif

The above library can be used as:

int main()
{
    openutils::mthread<int, std::pair<int, int>> thread;
    std::vector<std::pair<int, int>> args({{10, 20}, {5, 5}});

    thread.execute_processes({[](std::pair<int, int> x)
                              { return x.first + x.second; },
                              [](std::pair<int, int> x)
                              { return x.first - x.second; }},
                             args);

    std::vector<int> get = thread.get();

    for (const int &x : get)
        printf("%d\n", x);

    return 0;
}

Result:

30
0

Any suggestions, improvements are appreciated.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Read about: std::async \$\endgroup\$ May 7, 2023 at 6:33
  • \$\begingroup\$ Could've been a static function, returning ret_val instead of storing it somewhere... \$\endgroup\$ May 8, 2023 at 5:35

2 Answers 2

6
\$\begingroup\$

License

Just as a heads up.

// @license: This file is licensed under the GNU GENERAL PUBLIC LICENSE Version 3, 29 June 2007. You may obtain a copy of this license at https://www.gnu.org/licenses/gpl-3.0.en.html.

You have now all licensed this file under Creative Commons. There is a link at the bottom of the page. See: https://stackoverflow.com/help/licensing

Overview

OK. Nice try, if you are just experimenting.
BUT this is a bad idea.

I have a couple of issues with your implementation.

1: Threads are expensive to create.
2: There is no point in making more threads than you have processors. You are just doing busy work swapping between threads when you can simply spend time on one thread.

Given the above two. What you normally do is create a "Work" queue onto which you add work items. Simple interface run() the object has all the parameters embedded. Then you create a "Thread Pool", a set of thread objects(about the number of processors on your hardware). Each thread in the pool will execute a unique work item on the "Work" queue when finished it will get the next item or wait until another one is added.

Do a search for thread groups: Thread Pool in C++

But even thread pools are obsolete. The standard already provides an abstract mechanism to do this. std::async. Here you create an item of work and get a promise back. When the work item is finished the promise contains the result (or exception). Behind the sense the standard can create a thread pool for you.

CodeReview

The term MTHREAD seems very likely to be used previously. I would add the namespace into the include guard to try and make it more unique.

#ifndef MTHREAD_HH
#define MTHREAD_HH

The design of the class seems a bit odd.

    template <typename return_type, typename... args>
    class mthread
    {
    private:
        std::vector<return_type> ret_val;
        std::vector<std::thread> threads;

    public:
        void execute_processes(const std::vector<std::function<return_type(args...)>> &functions, const std::vector<args> &...a);
        const std::vector<return_type> &get() const;
        std::vector<return_type> &get();
    };

There is no constructor. But you start the execution by calling execute_processes() and passing all the values that are needed. Seems like this could be better designed as static method that simply returns the results.

So rather than:

thread.execute_processes({[](std::pair<int, int> x)
                          { return x.first + x.second; },
                          [](std::pair<int, int> x)
                          { return x.first - x.second; }},
                         args);

std::vector<int> get = thread.get();

I would want to use it like:

std::vector<int> get = 
mthread::execute_processes({[](std::pair<int, int> x)
                          { return x.first + x.second; },
                          [](std::pair<int, int> x)
                          { return x.first - x.second; }},
                         args);

Don't use this->

If you have to use this-> it means you have accidently shadowed a member variable with a local variable and you need to disambiguify the two members. The trouble is the compiler can not tell when you get it wrong.

So better solution is to always make your identifiers unique.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ "But even thread pools are obsolete. The standard already provides an abstract mechanism to do this. std::async." In the imperfect world we live in, std::async does not render thread pools obsolete - libstdc++, for example, has it launch a new thread for every call to it. However, future/promise interface can (and often is) utilized in modern thread pool libraries. \$\endgroup\$ May 7, 2023 at 23:08
  • 1
    \$\begingroup\$ Does any of the popular stdlib implementations use thread pools for std::async? \$\endgroup\$ May 8, 2023 at 7:15
  • 2
    \$\begingroup\$ "Don't use this->" not a C++ programmer but I see nothing wrong with using this->, actually, I prefer it because it's more "precise" i.e. it's always clear you're referencing an instance variable. \$\endgroup\$
    – Marco
    May 8, 2023 at 8:03
  • 1
    \$\begingroup\$ @Marco I agree with you as a C++ programmer, I don't see any valid reason to avoid this-> instead of someone's personal choice. If this-> is bad it should not exist in c++ compilers. \$\endgroup\$ May 8, 2023 at 12:47
  • 1
    \$\begingroup\$ @Darth-CodeX Just because the compiler can do something doesn't mean you should do it. See also: GOTO. There is a lot of historical backwards compatibility built into the compiler. The C++ standards body rarely removes capabilities with the general philosophy that any valid, compilable, standards-compliant code written at any point in time should still compile later on. I think Martin provides a very clear reason why it is potentially dangerous. "Accidently shadowed a variable" is one of those very painful, obscure bugs to solve. \$\endgroup\$
    – Drise
    May 8, 2023 at 17:16
3
\$\begingroup\$

To add to Martin York's answer:

Make it more generic

Your code works if the data is stored in std::vectors, but what if it was stored in a std::array, std::list or any other container type? You don't really care about the exact container type, just that you can iterate over them. Consider STL algorithms like std::transform(), or the C++20 ranges versions like std::ranges::transform(), except instead of applying one functions to a range of elements, you want to apply a range of functions on a range of elements. So modelling it on the standard library's algorithms, you could write something like:

template<std::ranges::input_range Input,
         std::ranges::weakly_incrementable Output,
         std::ranges::input_range Functions>
void parallel_transform(Input&& input, Output output, Functions&& functions) {
    std::vector<std::jthread> threads;
    auto function_it = functions.begin();
    for (auto& value: input) {
        output.emplace_back();
        threads.emplace_back([](const auto& value, auto& result, auto function) {
             result = std::invoke(function, value);
        }, std::cref(value), std::ref(output.back()), *function_it++);
    }
}

You'd use it like so:

std::vector<std::pair<int, int>> args({{10, 20}, {5, 5}});
std::vector<int> result;
std::vector<std::function<int(std::pair<int,int>)>> functions{
    [](std::pair<int, int> x){ return x.first + x.second; },
    [](std::pair<int, int> x){ return x.first - x.second; },
};

parallel_transform(args, std::back_inserter(result), functions);

for (int x: result)
    std::cout << x << '\n';

The small drawback is that you need to create the output container and a container of functions up front.

What about exceptions?

One problem you have with threads is that an exception can happen inside a thread. But how do you communicate that back to the caller? Martin York already hinted about std::async(). This functions solves the issue by returning a std::future, which will get notified of exceptions thrown in the async task. When calling get() on the std::future, the expection is rethrown, so the caller can then deal with it.

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