From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1881) id 6BFD93846410; Tue, 18 Aug 2020 14:30:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6BFD93846410 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1597761004; bh=KrZeSq34qbOtngoo6DuviT++a312lxhgIMzUKlBCg0Y=; h=From:To:Subject:Date:From; b=ylxZzkXa9ZU8ObfZy8xfOPcqBVlR1b2X8VXZcwItQc91BPCxYqVZhpo6sofd8LIp2 DfvKFxxGRrv33hSSoGeBql2lKGu+4Iz7daRl0dhDB9oPzHpdO3XmkADM2a9BKlPjGn PetN3syOBKGqrRv4NmZKNVy0OCe/JbO/CXpqEb+U= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Aaron Sawdey To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc(refs/users/acsawdey/heads/memcpy-unaligned-vsx)] libstdc++: Remove redundant copying of std::async arguments [PR 69724] X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/users/acsawdey/heads/memcpy-unaligned-vsx X-Git-Oldrev: 634046d1a81b48a48b3564ff2f90974f6b7b087b X-Git-Newrev: bb1b7f087bdd028000fd8f84e74b20adccc9d5bb Message-Id: <20200818143004.6BFD93846410@sourceware.org> Date: Tue, 18 Aug 2020 14:30:04 +0000 (GMT) X-BeenThere: libstdc++-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Aug 2020 14:30:04 -0000 https://gcc.gnu.org/g:bb1b7f087bdd028000fd8f84e74b20adccc9d5bb commit bb1b7f087bdd028000fd8f84e74b20adccc9d5bb Author: Jonathan Wakely 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. Diff: --- libstdc++-v3/include/std/future | 101 +++++++++---------- libstdc++-v3/include/std/thread | 32 ++---- libstdc++-v3/testsuite/30_threads/async/69724.cc | 119 +++++++++++++++++++++++ 3 files changed, 170 insertions(+), 82 deletions(-) diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future index bdf4a75d694..e0816c2f5e1 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -605,14 +605,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template class _Task_state; - template - static std::shared_ptr<_State_base> - _S_make_deferred_state(_BoundFn&& __fn); - - template - static std::shared_ptr<_State_base> - _S_make_async_state(_BoundFn&& __fn); - template struct _Task_setter; @@ -1614,10 +1606,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : public __future_base::_State_base { public: - explicit - _Deferred_state(_BoundFn&& __fn) - : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn)) - { } + template + explicit + _Deferred_state(_Args&&... __args) + : _M_result(new _Result<_Res>()), + _M_fn{{std::forward<_Args>(__args)...}} + { } private: typedef __future_base::_Ptr<_Result<_Res>> _Ptr_type; @@ -1679,69 +1673,63 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : public __future_base::_Async_state_commonV2 { public: - explicit - _Async_state_impl(_BoundFn&& __fn) - : _M_result(new _Result<_Res>()), _M_fn(std::move(__fn)) - { - _M_thread = std::thread{ [this] { - __try - { - _M_set_result(_S_task_setter(_M_result, _M_fn)); - } - __catch (const __cxxabiv1::__forced_unwind&) - { - // make the shared state ready on thread cancellation - if (static_cast(_M_result)) - this->_M_break_promise(std::move(_M_result)); - __throw_exception_again; - } - } }; - } + template + explicit + _Async_state_impl(_Args&&... __args) + : _M_result(new _Result<_Res>()), + _M_fn{{std::forward<_Args>(__args)...}} + { + _M_thread = std::thread{&_Async_state_impl::_M_run, this}; + } // Must not destroy _M_result and _M_fn until the thread finishes. // Call join() directly rather than through _M_join() because no other // thread can be referring to this state if it is being destroyed. - ~_Async_state_impl() { if (_M_thread.joinable()) _M_thread.join(); } + ~_Async_state_impl() + { + if (_M_thread.joinable()) + _M_thread.join(); + } private: + void + _M_run() + { + __try + { + _M_set_result(_S_task_setter(_M_result, _M_fn)); + } + __catch (const __cxxabiv1::__forced_unwind&) + { + // make the shared state ready on thread cancellation + if (static_cast(_M_result)) + this->_M_break_promise(std::move(_M_result)); + __throw_exception_again; + } + } + typedef __future_base::_Ptr<_Result<_Res>> _Ptr_type; _Ptr_type _M_result; _BoundFn _M_fn; }; - template - inline std::shared_ptr<__future_base::_State_base> - __future_base::_S_make_deferred_state(_BoundFn&& __fn) - { - typedef typename remove_reference<_BoundFn>::type __fn_type; - typedef _Deferred_state<__fn_type> __state_type; - return std::make_shared<__state_type>(std::move(__fn)); - } - - template - inline std::shared_ptr<__future_base::_State_base> - __future_base::_S_make_async_state(_BoundFn&& __fn) - { - typedef typename remove_reference<_BoundFn>::type __fn_type; - typedef _Async_state_impl<__fn_type> __state_type; - return std::make_shared<__state_type>(std::move(__fn)); - } - /// async template _GLIBCXX_NODISCARD future<__async_result_of<_Fn, _Args...>> async(launch __policy, _Fn&& __fn, _Args&&... __args) { + using _Wr = std::thread::_Call_wrapper<_Fn, _Args...>; + using _As = __future_base::_Async_state_impl<_Wr>; + using _Ds = __future_base::_Deferred_state<_Wr>; + std::shared_ptr<__future_base::_State_base> __state; if ((__policy & launch::async) == launch::async) { __try { - __state = __future_base::_S_make_async_state( - std::thread::__make_invoker(std::forward<_Fn>(__fn), - std::forward<_Args>(__args)...) - ); + __state = std::make_shared<_As>(std::forward<_Fn>(__fn), + std::forward<_Args>(__args)...); } #if __cpp_exceptions catch(const system_error& __e) @@ -1754,11 +1742,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } if (!__state) { - __state = __future_base::_S_make_deferred_state( - std::thread::__make_invoker(std::forward<_Fn>(__fn), - std::forward<_Args>(__args)...)); + __state = std::make_shared<_Ds>(std::forward<_Fn>(__fn), + std::forward<_Args>(__args)...); } - return future<__async_result_of<_Fn, _Args...>>(__state); + return future<__async_result_of<_Fn, _Args...>>(std::move(__state)); } /// async, potential overload diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread index 30ae93a0d5b..887ee579962 100644 --- a/libstdc++-v3/include/std/thread +++ b/libstdc++-v3/include/std/thread @@ -145,11 +145,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #else auto __depend = nullptr; #endif - // 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)...), + using _Wrapper = _Call_wrapper<_Callable, _Args...>; + // Create a call wrapper with DECAY_COPY(__f) as its target object + // and DECAY_COPY(__args)... as its bound argument entities. + _M_start_thread(_State_ptr(new _State_impl<_Wrapper>( + std::forward<_Callable>(__f), std::forward<_Args>(__args)...)), __depend); } @@ -220,13 +220,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _M_start_thread(_State_ptr, void (*)()); - template - static _State_ptr - _S_make_state(_Args&&... __args) - { - using _Impl = _State_impl<_Callable>; - return _State_ptr{new _Impl{std::forward<_Args>(__args)...}}; - } #if _GLIBCXX_THREAD_ABI_COMPAT public: struct _Impl_base; @@ -274,20 +267,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } }; - template - using __decayed_tuple = tuple::type...>; - public: - // Returns a call wrapper that stores - // tuple{DECAY_COPY(__callable), DECAY_COPY(__args)...}. - template - static _Invoker<__decayed_tuple<_Callable, _Args...>> - __make_invoker(_Callable&& __callable, _Args&&... __args) - { - return { __decayed_tuple<_Callable, _Args...>{ - std::forward<_Callable>(__callable), std::forward<_Args>(__args)... - } }; - } + template + using _Call_wrapper = _Invoker::type...>>; }; inline void diff --git a/libstdc++-v3/testsuite/30_threads/async/69724.cc b/libstdc++-v3/testsuite/30_threads/async/69724.cc new file mode 100644 index 00000000000..0771d638cd1 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/async/69724.cc @@ -0,0 +1,119 @@ +// Copyright (C) 2019-2020 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 +// . + +// { dg-do run } +// { dg-options "-pthread" } +// { dg-require-effective-target c++11 } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } + +#include +#include + +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::future fut; + + F::copycount = 0; + fut = std::async(&F::run, F{}); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); + + F::copycount = 0; + fut = std::async(std::launch::async, &F::run, F{}); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); + + F::copycount = 0; + fut = std::async(std::launch::deferred, &F::run, F{}); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); +} + +void +test02() +{ + std::future fut; + const F f; + + F::copycount = 0; + fut = std::async(&F::run, f); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); + + F::copycount = 0; + fut = std::async(std::launch::async, &F::run, f); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); + + F::copycount = 0; + fut = std::async(std::launch::deferred, &F::run, f); + VERIFY( F::copycount == 1 ); + fut.get(); + VERIFY( F::copycount == 1 ); +} + +void +test03() +{ + std::future fut; + F f; + + F::copycount = 0; + fut = std::async(&F::run, std::ref(f)); + VERIFY( F::copycount == 0 ); + fut.get(); + VERIFY( F::copycount == 0 ); + + F::copycount = 0; + fut = std::async(std::launch::async, &F::run, std::ref(f)); + VERIFY( F::copycount == 0 ); + fut.get(); + VERIFY( F::copycount == 0 ); + + F::copycount = 0; + fut = std::async(std::launch::deferred, &F::run, std::ref(f)); + VERIFY( F::copycount == 0 ); + fut.get(); + VERIFY( F::copycount == 0 ); +} + +int +main() +{ + test01(); + test02(); + test03(); +}