* Re: [patch] Avoid memory allocations when making futures ready
@ 2014-08-07 22:07 Dominique Dhumieres
2014-08-07 22:09 ` Jonathan Wakely
2014-08-07 22:55 ` Jonathan Wakely
0 siblings, 2 replies; 4+ messages in thread
From: Dominique Dhumieres @ 2014-08-07 22:07 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++, jwakely
> This patch replaces some reference members with pointers, ...
> ...
> Tested x86_64-linux, committed to trunk.
This breaks bootstrap (see https://gcc.gnu.org/ml/gcc-regression/2014-08/):
...
libtool: compile: /opt/gcc/build_w/./gcc/xgcc -shared-libgcc -B/opt/gcc/build_w/./gcc -nostdinc++ -L/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/src -L/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/src/.libs -L/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/libsupc++/.libs -B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/bin/ -B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/lib/ -isystem /opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/include -isystem /opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/sys-include -I/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/x86_64-apple-darwin13.3.0 -I/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include -I/opt/gcc/work/libstdc++-v3/libsupc++ -fno-common -DPIC -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -fvisibility-inlines-hidden -ffunction-sections -fdata-sections -frandom-seed=compatibility-thread-c++0x.lo -g -O2 -std=gnu++11 -c ../../../../work/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc -fno-common -DPIC -D_GLIBCXX_SHARED -o .libs/compatibility-thread-c++0x.o
In file included from ../../../../work/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:30:0:
/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/future:557:6: error: incomplete type 'std::__future_base::_State_base' used in nested name specifier
<__future_base::_State_base::_Setter<_Res, _Arg>>
^
/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/future:557:6: error: incomplete type 'std::__future_base::_State_base' used in nested name specifier
/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/future:557:48: error: wrong number of template arguments (2, should be 1)
<__future_base::_State_base::_Setter<_Res, _Arg>>
^
In file included from /opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/future:38:0,
from ../../../../work/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:30:
/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/functional:1749:12: error: provided for 'template<class _Tp> struct std::__is_location_invariant'
struct __is_location_invariant
^
In file included from ../../../../work/libstdc++-v3/src/c++11/compatibility-thread-c++0x.cc:30:0:
/opt/gcc/build_w/x86_64-apple-darwin13.3.0/libstdc++-v3/include/future:557:52: error: expected unqualified-id before '>' token
<__future_base::_State_base::_Setter<_Res, _Arg>>
^
make[6]: *** [compatibility-thread-c++0x.lo] Error 1
...
TIA
Dominique
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] Avoid memory allocations when making futures ready
2014-08-07 22:07 [patch] Avoid memory allocations when making futures ready Dominique Dhumieres
@ 2014-08-07 22:09 ` Jonathan Wakely
2014-08-07 22:55 ` Jonathan Wakely
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2014-08-07 22:09 UTC (permalink / raw)
To: Dominique Dhumieres; +Cc: gcc-patches, libstdc++, Jonathan Wakely
On 7 August 2014 23:07, Dominique Dhumieres wrote:
>> This patch replaces some reference members with pointers, ...
>> ...
>> Tested x86_64-linux, committed to trunk.
>
> This breaks bootstrap (see https://gcc.gnu.org/ml/gcc-regression/2014-08/):
Strange, I didn't see that - I'll fix it ASAP.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] Avoid memory allocations when making futures ready
2014-08-07 22:07 [patch] Avoid memory allocations when making futures ready Dominique Dhumieres
2014-08-07 22:09 ` Jonathan Wakely
@ 2014-08-07 22:55 ` Jonathan Wakely
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2014-08-07 22:55 UTC (permalink / raw)
To: Dominique Dhumieres; +Cc: gcc-patches, libstdc++
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On 08/08/14 00:07 +0200, Dominique Dhumieres wrote:
>> This patch replaces some reference members with pointers, ...
>> ...
>> Tested x86_64-linux, committed to trunk.
>
>This breaks bootstrap (see https://gcc.gnu.org/ml/gcc-regression/2014-08/):
Fixed by this patch, sorry about that. I was caught out (again) by the
files in libstdc++-v3/src/*/ not being recompiled when headers change.
Tested x86_64-linux with full bootstrap, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1015 bytes --]
commit b5af1e35e9c52ea8738997431ab958dc4caf85c6
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Aug 7 23:20:09 2014 +0100
* include/std/future (__location_invariant): Move specializations
after preprocessor condition.
diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index c3aaaec..8989474 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -551,6 +551,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void _M_destroy() { delete this; }
};
+#ifndef _GLIBCXX_ASYNC_ABI_COMPAT
+
// Allow _Setter objects to be stored locally in std::function
template<typename _Res, typename _Arg>
struct __is_location_invariant
@@ -563,8 +565,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
<__future_base::_Task_setter<_Res_ptr, _Fn, _Res>>
: true_type { };
-#ifndef _GLIBCXX_ASYNC_ABI_COMPAT
-
/// Common implementation for future and shared_future.
template<typename _Res>
class __basic_future : public __future_base
^ permalink raw reply [flat|nested] 4+ messages in thread
* [patch] Avoid memory allocations when making futures ready
@ 2014-08-07 20:13 Jonathan Wakely
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2014-08-07 20:13 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 449 bytes --]
This patch replaces some reference members with pointers, so that the
types containing them become trivial and are more efficient to pass as
function arguments and can use the small object optimization when
stored in a std::function, so that promise::set_value() doesn't need
to allocate any memory.
_Task_setter is also changed to store a pointer to the task, instead
of using a std::function to hold it.
Tested x86_64-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 10559 bytes --]
commit 09c2fd279f76b886efa8bdb0b9b2dcb0956ab7f5
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Sat May 17 14:09:40 2014 +0100
* include/std/future (_State_baseV2::_M_set_result): Pass pointers to
_M_do_set.
(_State_baseV2::_M_do_set): Change parameters to pointers.
(_State_baseV2::_Setter): Change _M_arg from reference to pointer.
(_State_baseV2::__setter): Initialize _Setter with pointers.
(_State_baseV2::__setter(promise<void>*)): Remove overload.
(promise::set_value, promise::set_exception): Pass setter directly
to _M_set_result.
(_State_baseV2::_Task_setter): Add template parameter for callable
type and replace std::function member with pointer to that type.
Change _M_result member from reference to pointer.
(_State_baseV2::_S_task_setter): Change parameter to lvalue reference
and initialize _Task_setter with pointers.
(__location_invariant): Specialize for _Setter and _Task_setter.
diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index be2ed96..c3aaaec 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -358,7 +358,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// all calls to this function are serialized,
// side-effects of invoking __res only happen once
call_once(_M_once, &_State_baseV2::_M_do_set, this,
- ref(__res), ref(__lock));
+ std::__addressof(__res), std::__addressof(__lock));
if (__lock.owns_lock())
_M_cond.notify_all();
else if (!__ignore_failure)
@@ -396,19 +396,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct _Setter<_Res, _Arg&>
{
// check this is only used by promise<R>::set_value(const R&)
- // or promise<R>::set_value(R&)
+ // or promise<R&>::set_value(R&)
static_assert(is_same<_Res, _Arg&>::value // promise<R&>
- || is_same<const _Res, _Arg>::value, // promise<R>
+ || is_same<const _Res, _Arg>::value, // promise<R>
"Invalid specialisation");
typename promise<_Res>::_Ptr_type operator()()
{
_State_baseV2::_S_check(_M_promise->_M_future);
- _M_promise->_M_storage->_M_set(_M_arg);
+ _M_promise->_M_storage->_M_set(*_M_arg);
return std::move(_M_promise->_M_storage);
}
promise<_Res>* _M_promise;
- _Arg& _M_arg;
+ _Arg* _M_arg;
};
// set rvalues
@@ -418,11 +418,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename promise<_Res>::_Ptr_type operator()()
{
_State_baseV2::_S_check(_M_promise->_M_future);
- _M_promise->_M_storage->_M_set(std::move(_M_arg));
+ _M_promise->_M_storage->_M_set(std::move(*_M_arg));
return std::move(_M_promise->_M_storage);
}
promise<_Res>* _M_promise;
- _Res& _M_arg;
+ _Res* _M_arg;
};
struct __exception_ptr_tag { };
@@ -434,31 +434,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
typename promise<_Res>::_Ptr_type operator()()
{
_State_baseV2::_S_check(_M_promise->_M_future);
- _M_promise->_M_storage->_M_error = _M_ex;
+ _M_promise->_M_storage->_M_error = *_M_ex;
return std::move(_M_promise->_M_storage);
}
promise<_Res>* _M_promise;
- exception_ptr& _M_ex;
+ exception_ptr* _M_ex;
};
template<typename _Res, typename _Arg>
static _Setter<_Res, _Arg&&>
__setter(promise<_Res>* __prom, _Arg&& __arg)
{
- return _Setter<_Res, _Arg&&>{ __prom, __arg };
+ return _Setter<_Res, _Arg&&>{ __prom, &__arg };
}
template<typename _Res>
static _Setter<_Res, __exception_ptr_tag>
__setter(exception_ptr& __ex, promise<_Res>* __prom)
{
- return _Setter<_Res, __exception_ptr_tag>{ __prom, __ex };
+ return _Setter<_Res, __exception_ptr_tag>{ __prom, &__ex };
}
- static _Setter<void, void>
- __setter(promise<void>* __prom);
-
template<typename _Tp>
static void
_S_check(const shared_ptr<_Tp>& __p)
@@ -469,10 +466,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
private:
void
- _M_do_set(function<_Ptr_type()>& __f, unique_lock<mutex>& __lock)
+ _M_do_set(function<_Ptr_type()>* __f, unique_lock<mutex>* __lock)
{
- _Ptr_type __res = __f(); // do not hold lock while running setter
- __lock.lock();
+ _Ptr_type __res = (*__f)(); // do not hold lock while running setter
+ __lock->lock();
_M_result.swap(__res);
}
@@ -514,15 +511,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static std::shared_ptr<_State_base>
_S_make_async_state(_BoundFn&& __fn);
- template<typename _Res_ptr,
+ template<typename _Res_ptr, typename _Fn,
typename _Res = typename _Res_ptr::element_type::result_type>
struct _Task_setter;
template<typename _Res_ptr, typename _BoundFn>
- static _Task_setter<_Res_ptr>
- _S_task_setter(_Res_ptr& __ptr, _BoundFn&& __call)
+ static _Task_setter<_Res_ptr, _BoundFn>
+ _S_task_setter(_Res_ptr& __ptr, _BoundFn& __call)
{
- return _Task_setter<_Res_ptr>{ __ptr, std::ref(__call) };
+ return { std::__addressof(__ptr), std::__addressof(__call) };
}
};
@@ -554,6 +551,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void _M_destroy() { delete this; }
};
+ // Allow _Setter objects to be stored locally in std::function
+ template<typename _Res, typename _Arg>
+ struct __is_location_invariant
+ <__future_base::_State_base::_Setter<_Res, _Arg>>
+ : true_type { };
+
+ // Allow _Task_setter objects to be stored locally in std::function
+ template<typename _Res_ptr, typename _Fn, typename _Res>
+ struct __is_location_invariant
+ <__future_base::_Task_setter<_Res_ptr, _Fn, _Res>>
+ : true_type { };
+
#ifndef _GLIBCXX_ASYNC_ABI_COMPAT
/// Common implementation for future and shared_future.
@@ -994,24 +1003,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Setting the result
void
set_value(const _Res& __r)
- {
- auto __setter = _State::__setter(this, __r);
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(this, __r)); }
void
set_value(_Res&& __r)
- {
- auto __setter = _State::__setter(this, std::move(__r));
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(this, std::move(__r))); }
void
set_exception(exception_ptr __p)
- {
- auto __setter = _State::__setter(__p, this);
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(__p, this)); }
};
template<typename _Res>
@@ -1092,17 +1092,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Setting the result
void
set_value(_Res& __r)
- {
- auto __setter = _State::__setter(this, __r);
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(this, __r)); }
void
set_exception(exception_ptr __p)
- {
- auto __setter = _State::__setter(__p, this);
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(__p, this)); }
};
/// Explicit specialization for promise<void>
@@ -1177,10 +1171,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void
set_exception(exception_ptr __p)
- {
- auto __setter = _State::__setter(__p, this);
- _M_future->_M_set_result(std::move(__setter));
- }
+ { _M_future->_M_set_result(_State::__setter(__p, this)); }
};
// set void
@@ -1196,28 +1187,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
promise<void>* _M_promise;
};
- inline __future_base::_State_base::_Setter<void, void>
- __future_base::_State_base::__setter(promise<void>* __prom)
- {
- return _Setter<void, void>{ __prom };
- }
-
inline void
promise<void>::set_value()
- {
- auto __setter = _State::__setter(this);
- _M_future->_M_set_result(std::move(__setter));
- }
-
+ { _M_future->_M_set_result(_State::_Setter<void, void>{ this }); }
- template<typename _Ptr_type, typename _Res>
+ template<typename _Ptr_type, typename _Fn, typename _Res>
struct __future_base::_Task_setter
{
_Ptr_type operator()()
{
__try
{
- _M_result->_M_set(_M_fn());
+ (*_M_result)->_M_set((*_M_fn)());
}
__catch(const __cxxabiv1::__forced_unwind&)
{
@@ -1225,22 +1206,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
__catch(...)
{
- _M_result->_M_error = current_exception();
+ (*_M_result)->_M_error = current_exception();
}
- return std::move(_M_result);
+ return std::move(*_M_result);
}
- _Ptr_type& _M_result;
- std::function<_Res()> _M_fn;
+ _Ptr_type* _M_result;
+ _Fn* _M_fn;
};
- template<typename _Ptr_type>
- struct __future_base::_Task_setter<_Ptr_type, void>
+ template<typename _Ptr_type, typename _Fn>
+ struct __future_base::_Task_setter<_Ptr_type, _Fn, void>
{
_Ptr_type operator()()
{
__try
{
- _M_fn();
+ (*_M_fn)();
}
__catch(const __cxxabiv1::__forced_unwind&)
{
@@ -1248,12 +1229,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
__catch(...)
{
- _M_result->_M_error = current_exception();
+ (*_M_result)->_M_error = current_exception();
}
- return std::move(_M_result);
+ return std::move(*_M_result);
}
- _Ptr_type& _M_result;
- std::function<void()> _M_fn;
+ _Ptr_type* _M_result;
+ _Fn* _M_fn;
};
template<typename _Res, typename... _Args>
@@ -1294,8 +1275,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// bound arguments decay so wrap lvalue references
auto __boundfn = std::__bind_simple(std::ref(_M_impl._M_fn),
_S_maybe_wrap_ref(std::forward<_Args>(__args))...);
- auto __setter = _S_task_setter(this->_M_result, std::move(__boundfn));
- this->_M_set_result(std::move(__setter));
+ this->_M_set_result(_S_task_setter(this->_M_result, __boundfn));
}
virtual shared_ptr<_Task_state_base<_Res(_Args...)>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-08-07 22:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07 22:07 [patch] Avoid memory allocations when making futures ready Dominique Dhumieres
2014-08-07 22:09 ` Jonathan Wakely
2014-08-07 22:55 ` Jonathan Wakely
-- strict thread matches above, loose matches on Subject: below --
2014-08-07 20:13 Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).