[PATCH] PR libstdc++/69724 avoid temporary in std::thread construction

[Jonathan Wakely]

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.

Tested powerpc64le-linux, committed to trunk.

I'll leave the bug report open as I want to do a similar optimization
for std::async's DECAY_COPY as well.


-------------- next part --------------
commit fddee59c34cbdc4872f65deebf5772cff9845fc7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Mar 14 16:41:52 2019 +0000

    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.

diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 2da1101eecf..90b4be6cd16 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -127,9 +127,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
 	auto __depend = nullptr;
 #endif
-        _M_start_thread(_S_make_state(
-	      __make_invoker(std::forward<_Callable>(__f),
-			     std::forward<_Args>(__args)...)),
+	// A call wrapper holding tuple{DECAY_COPY(__f), DECAY_COPY(__args)...}
+	using _Invoker_type = _Invoker<__decayed_tuple<_Callable, _Args...>>;
+
+	_M_start_thread(_S_make_state<_Invoker_type>(
+	      std::forward<_Callable>(__f), std::forward<_Args>(__args)...),
 	    __depend);
       }
 
@@ -188,8 +190,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	_Callable		_M_func;
 
-	_State_impl(_Callable&& __f) : _M_func(std::forward<_Callable>(__f))
-	{ }
+	template<typename... _Args>
+	  _State_impl(_Args&&... __args)
+	  : _M_func{{std::forward<_Args>(__args)...}}
+	  { }
 
 	void
 	_M_run() { _M_func(); }
@@ -198,12 +202,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     void
     _M_start_thread(_State_ptr, void (*)());
 
-    template<typename _Callable>
+    template<typename _Callable, typename... _Args>
       static _State_ptr
-      _S_make_state(_Callable&& __f)
+      _S_make_state(_Args&&... __args)
       {
 	using _Impl = _State_impl<_Callable>;
-	return _State_ptr{new _Impl{std::forward<_Callable>(__f)}};
+	return _State_ptr{new _Impl{std::forward<_Args>(__args)...}};
       }
 #if _GLIBCXX_THREAD_ABI_COMPAT
   public:
diff --git a/libstdc++-v3/testsuite/30_threads/thread/cons/69724.cc b/libstdc++-v3/testsuite/30_threads/thread/cons/69724.cc
new file mode 100644
index 00000000000..ee3a8602b29
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/thread/cons/69724.cc
@@ -0,0 +1,70 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run }
+// { dg-options "-pthread"  }
+// { dg-require-effective-target c++11 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+#include <thread>
+#include <testsuite_rvalref.h>
+
+struct F : __gnu_test::copycounter
+{
+  F() = default;
+
+  F(const F&) = default;
+
+  // Move constructor copies base class, to use counter:
+  F(F&& f) : copycounter(f) { f.valid = false; }
+
+  void run() { VERIFY(this->valid); }
+};
+
+void
+test01()
+{
+  std::thread{&F::run, F{}}.join();
+  VERIFY( F::copycount == 1 );
+}
+
+void
+test02()
+{
+  F::copycount = 0;
+  const F f;
+  std::thread{&F::run, f}.join();
+  VERIFY( F::copycount == 1 );
+}
+
+void
+test03()
+{
+  F::copycount = 0;
+  F f;
+  std::thread{&F::run, std::ref(f)}.join();
+  VERIFY( F::copycount == 0 );
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  test03();
+}