Bug 69724 - Unnecessary temporary object during std::thread construction
Summary: Unnecessary temporary object during std::thread construction
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 6.0
: P3 enhancement
Target Milestone: 11.0
Assignee: Jonathan Wakely
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2016-02-09 00:31 UTC by Jonathan Wakely
Modified: 2020-08-18 13:30 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-01-18 00:00:00


Attachments
Patch for next stage 1 (858 bytes, patch)
2019-03-14 16:48 UTC, Jonathan Wakely
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2016-02-09 00:31:58 UTC
#include <thread>
#include <cstdio>

class F {
public:
    F() { std::puts("create"); }
    ~F() { std::puts("destroy"); }
    F(const F&) { std::puts("copy"); }
    F(F&&) noexcept { std::puts("move"); }
    void run() { }
};

int main()
{
  std::thread{&F::run, F{}}.join();
}

prints:

create
move
move
destroy
destroy
destroy

The standard requires a copy to be made, so one of the move constructions is required, but a more efficient result would be:

create
move
destroy
destroy

This could be done by removing the call to __bind_simple() and constructing the _Bind_simple member of the thread::_State_impl directly, rather than returning by value from __bind_simple() and then moving that value into the member.

However, the benefit for most types is probably small so the additional complexity might not be worth it.
Comment 1 Jonathan Wakely 2017-01-18 17:04:24 UTC
On trunk __bind_simple has been replaced by thread::__make_invoker but the number of move constructions is still the same.
Comment 2 Jonathan Wakely 2019-03-14 16:48:04 UTC
Created attachment 45968 [details]
Patch for next stage 1
Comment 3 Jonathan Wakely 2019-03-14 16:49:05 UTC
thread::__make_invoker is still used by std::async, but a similar optimization could be done for __future_base::_S_make_async_state.
Comment 4 Jonathan Wakely 2019-05-14 12:01:46 UTC
Author: redi
Date: Tue May 14 12:01:15 2019
New Revision: 271166

URL: https://gcc.gnu.org/viewcvs?rev=271166&root=gcc&view=rev
Log:
PR libstdc++/69724 avoid temporary in std::thread construction

The std::thread constructor creates (and then moves) an unnecessary
temporary copy of each argument. Optimize it to only make the one copy
that is required.

	PR libstdc++/69724
	* include/std/thread (thread::_State_impl, thread::_S_make_state):
	Replace single _Callable parameter with variadic _Args pack, to
	forward them directly to the tuple of decayed copies.
	* testsuite/30_threads/thread/cons/69724.cc: New test.

Added:
    trunk/libstdc++-v3/testsuite/30_threads/thread/cons/69724.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/thread
Comment 5 Jonathan Wakely 2019-05-14 12:14:39 UTC
I'm leaving this open until std::async is changed too.
Comment 6 Jonathan Wakely 2020-01-17 17:14:33 UTC
(In reply to Jonathan Wakely from comment #5)
> I'm leaving this open until std::async is changed too.

Which isn't in scope for GCC 10 now.
Comment 7 CVS Commits 2020-08-18 13:28:52 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:bb1b7f087bdd028000fd8f84e74b20adccc9d5bb

commit r11-2736-gbb1b7f087bdd028000fd8f84e74b20adccc9d5bb
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Aug 18 14:23:19 2020 +0100

    libstdc++: Remove redundant copying of std::async arguments [PR 69724]
    
    As was previously done for std::thread, this removes an unnecessary copy
    of an rvalue of type thread::_Invoker. Instead of creating the rvalue
    and then moving that into the shared state, the member of the shared
    state is initialized directly from the forwarded callable and bound
    arguments.
    
    This also slightly simplifies std::thread creation to remove the
    _S_make_state helper function.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/69724
            * include/std/future (__future_base::_S_make_deferred_state)
            (__future_base::_S_make_async_state): Remove.
            (__future_base::_Deferred_state): Change constructor to accept a
            parameter pack of arguments and forward them to the call
            wrapper.
            (__future_base::_Async_state_impl): Likewise. Replace lambda
            expression with a named member function.
            (async): Construct state object directly from the arguments,
            instead of using thread::__make_invoker, _S_make_deferred_state
            and _S_make_async_state. Move shared state into the returned
            future.
            * include/std/thread (thread::_Call_wrapper): New alias
            template for use by constructor and std::async.
            (thread::thread(Callable&&, Args&&...)): Create state object
            directly instead of using _S_make_state.
            (thread::__make_invoker, thread::__decayed_tuple)
            (thread::_S_make_state): Remove.
            * testsuite/30_threads/async/69724.cc: New test.
Comment 8 Jonathan Wakely 2020-08-18 13:30:19 UTC
std::thread was fixed for GCC 10.1 and std::async is fixed for GCC 11.