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.

From-SVN: r271166

3 files changed, 90 insertions, 8 deletions

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 9a90e1b..995f2ce 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,11 @@
+2019-05-14  Jonathan Wakely  <jwakely@redhat.com>
+
+	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.
+
 2019-05-14  Nina Dinka Ranns  <dinka.ranns@gmail.com>
 
 	Inconsistency wrt Allocators in basic_string assignment (LWG2579)
diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 2da1101..90b4be6 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 0000000..ee3a860
--- /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();
+}