How std::packaged_task works

Question

I am analysing the following code snippet and am trying to understand it in detail:

template<typename FUNCTION, typename... ARGUMENTS>
auto ThreadPool::add( FUNCTION&& Function, ARGUMENTS&&... Arguments ) -> std::future<typename std::result_of<FUNCTION(ARGUMENTS...)>::type>
{
    using PackedTask = std::packaged_task<typename std::result_of<FUNCTION(ARGUMENTS...)>::type()>;

    auto task = std::make_shared<PackedTask>(std::bind(std::forward<FUNCTION>(Function), std::forward<ARGUMENTS>(Arguments)...));

    // get the future to return later
    auto ret = task->get_future();

    {
        std::lock_guard<std::mutex> lock{jobsMutex};
        jobs.emplace([task]() { (*task)(); });
    }

    // let a waiting thread know there is an available job
    jobsAvailable.notify_one();

    return ret;
}

I have few questions regarding the std::packaged_task.

As you can see in the add(...) method body, the instance of std::packaged_task - task is local variable which scope ends with the end of the method execution. The return value ret of std::future type is returned by copy. The value of ret is given out of the task (which is local). So once the method execution is finished, the task goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?

During the execution of the method the task method to be executed in a thread is emplaced into std::queue<Job> jobs. Why is only the pointer to operator() of std::packaged_task which holds the reference to the Function given as method argument held in the std::queue? I would expect to store directly the std::packaged_task in order to hold the reference to the instance being created...?

Anyway, the source code snippet comes from the ThreadPool implementation which can be found here https://github.com/dabbertorres/ThreadPool and seems to be fully working. So I assume it is correct but unfortunately not completely clear to me... I would be very glad if someone could explain how the stuff works...

Many thanks in advance to anyone willing to help. Cheers Martin

Placia

Just a note, the result-type deduction is seems off. The actual call forwards the universal references, whereas the std::result_of does not. – Levator

@Levator Maybe a stupid question but what does it mean? So you propose not to use std::result_of and replace it somehow? Could you please post a sample code reflecting this change? – Schertz

I'd expect decltype(std::forward<F>(f(std::forward<A>(a)...)) or similar – Levator

Answer 1

As you can see in the add(...) method body, the instance of std::packaged_task - task is local variable which scope ends with the end of the method execution.

yes, it's a local variable of std::shared_ptr<PackedTask>. when it gets out of scope, it decrements the reference count by 1. if the new reference count is 0, it deletes the object it points to. luckily, jobs holds a copy of that shared pointer, so that pointed-object stays alive.

The return value ret of std::future type is returned by copy.

Not exactly. it's more possible that ret is returned by move rather than by copy.

the task goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?

Again, task is a shared pointer. the jobs queue keeps it alive and probably ret holds another copy of that shared pointer (to actually pull the result of task). so nothing becomes invalid as long as the task is in the queue and someone holds the future to that result.

I would expect to store directly the std::packaged_task in order to hold the reference to the instance being created...?

True, it could have stored std::packaged_task directly. my guess is that the writer of this library didn't want to mess with uncopiable/unmovable functors. if the result of std::bind is uncopiable and unmovable, you can't really move the std::packaged_task into the queue. using a shared pointer solves this problem as you don't copy/move the packaged_task itself, only the pointer. you can however build the the task object directly into the queue, but doing it under a held lock is not such a good practice.

I do agree however, that maybe moving task into the lambda is way more efficient than copying it.

Werra

could you suppose in this code why and what does line: jobs.emplace([task]() { (*task)(); }) add? I mean could you explain how (and what is a reason here) packaed_task is converted to std::function<void()> ? jobs really stores these objects. the only reason I can see here - is that to reduce return type to void of the callable object – Confectioner

the lambda turns into a std::function, not the packaged_task. – Werra

I see but i dont understand the using of std::function at all. Why it cant be used directly? I mean to have a queue of packed tasks? – Confectioner

@Confectioner Because the semantics of std::function do not provide the features needed by this implementation. A packaged_task is more flexible (although could be overkill), it bundles together a future a promise and a function. When the thread finishes the task it is "synchronized with" the return value and the internal promise's set_value() is set by the packaged_task and then the caller can get the return value by future.get(). You could do with std::function but requires a different architecture. You could do with just a future and a promise, but a packaged_task is flexible. – Dialectician

@Confectioner You cannot have a queue of packaged tasks where each element is generally a different instance of this template. Note that the created instances of packaged tasks encode information about return types for jobs in their template argument. Without the return type, you couldn't create futures for tasks. As a summary, packaged tasks are used for obtaining futures for jobs, and std::function is employed to store jobs into the queue. – Hammon

It's more possible that ret is returned by move rather than by copy. — This is guaranteed, ret is treated as rvalue in return statement. Otherwise, the code wouldn't compile since std::future is not copyable. – Hammon

@DanielLangr thanks! actually I've asked own question about. here is #49810944 . anyway , thank you ! – Confectioner

@Confectioner You're welcome, will look there. Note that, even if users were able to define tasks completely without parameters and return values, you still need to care about exceptions. This is where packaged_task and future are of great help. But you're right that without different return types, the queue inside a thread pool could just contain std::packaged_task<void()> directly. It's basically a design decision. – Hammon

Answer 2

So once the method execution is finished, the task goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?

task is a std::shared_ptr that is copied into the closure passed to jobs - it will be kept alive for as long as the job lives.

Stickup

And that's pretty inefficient. At least with c++14 they could just move the task in there instead of dynamically allocating it and keeping a refcount too. – Levator

@sehe: and without C++14 they could just use an old good struct :) – Stickup

Guys, many thanks for your valuable comments and hints! Maybe, not for me only, it would be very helpfull and well appreciated - could you change the snippet to reflect all your optimizations coming using C++14? So the std::queue would be of std::packaged_task while the instances would be moved there (by std::move) as the std::packaged_task is not copiable? Do I get you right? – Schertz

@VittorioRomeo it took me half a minute to realize you meant instead of the lambda, not instead of the move-only type. (We call those calleable types where I live :)) – Levator

@sehe, from my experience, holding a synchronized queue/array of pointers is much more efficient than array/queue of objects in a highly contented multithreaded application. if the queue/array needs to re-manage its own internal state do to reallocations and stuff, copying pointers is much more efficient than start moving objects around. remember anything should be under a locked-lock. also, building an object outside the synchronized block and pushing only its pointer is much more efficient than building it directly into the queue while the lock is locked. the same goes for pulling. – Werra

@Levator as for the overhead of memory allocations, a simple memory pool can shave off most of the overhead of the memory allocations. this is what I did in my own implementation of futures/promises : github.com/David-Haim/concurrencpp . believe me, I did my benchmarking :) – Werra

@DavidHaim what problem does that solve? I though we already established no allocation was required beyond the one managed by packaged_task (which may very well have been optimized) – Levator

@Levator I am trying to rewrite my code reflecting all your well appreciated comments. But how to declare the queue hodling all the packaged_task pointers? I am facing the trouble the packaged_task type (shared pointer to task) is determined in the add(..) method body so in general is always different. So if I want to have a std::queue<std::shared_ptr<std::packaged_task<???>>> what type of packaaged_task it must be? – Schertz

The way I see it, the underlying queue just takes std::function<void()>? (Hence jobs.emplace([task]() { (*task)(); });). The bound object stores the shared packaged-task internally (which is why I think it need not be shared) – Levator

@DavidHaim: could you please share the benchmarks? Greatly interested in them. – Stickup

Answer 3

So once the method execution is finished, the task goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?

No.

The future is a handle to shared state. This state is shared by the promise and the future.

Think of it as encapsulating a special kind of shared_ptr to the (opaque) object which manages the state of the promise/future relationship.

Sally