Commit 68cf03b1 authored by Giuseppe Ottaviano's avatar Giuseppe Ottaviano Committed by Facebook Github Bot 3

Further Function simplification

Summary:
Use tag dispatching instead of `enable_if`: it is clearer, it
sidesteps the GCC mangling bug, and more importantly the conditional
doesn't leak into the symbol, making stack traces and profiles more
readable.

Testing on a compilation unit with 1000 `Function`s from simple lambdas.
Before:
```
folly::impl::Function<int (), false>::Function<main::{lambda()#1}, {lambda()#1}>(main::{lambda()#1}&&, std::enable_if<std::integral_constant<bool, ((sizeof (std::decay<main::{lambda()#1}>::type))<=(sizeof folly::detail::function::Data::small))&&std::is_nothrow_move_constructible<std::decay<main::{lambda()#1}> >::value>::value, folly::detail::Tag>::type)::Ops::call(folly::detail::function&)
```
After:
```
folly::impl::Function<int (), false>::OpsSmall<main::{lambda()#1}>::call(folly::detail::function::Data&)
```
Note that the function type is repeated 5 times before, and only once after. This makes a large difference with long namespaces.

Binary size is almost unaffected, compile times slightly better:
Before:
GCC opt: 22.3 seconds, 4435232 bytes
Clang dev: 7.7 seconds, 5257344 bytes

After:
GCC opt: 18.6 seconds, 4493920 bytes
Clang dev: 7.2 seconds, 5469136 bytes

Reviewed By: ericniebler

Differential Revision: D3231530

fb-gh-sync-id: 6aa76e7f780a8afdbfed8a378f257ceb86dce704
fbshipit-source-id: 6aa76e7f780a8afdbfed8a378f257ceb86dce704
parent 33344a23
...@@ -246,8 +246,6 @@ union Data { ...@@ -246,8 +246,6 @@ union Data {
typename std::aligned_storage<6 * sizeof(void*)>::type small; typename std::aligned_storage<6 * sizeof(void*)>::type small;
}; };
struct Tag {};
template <bool If, typename T> template <bool If, typename T>
using ConstIf = typename std::conditional<If, const T, T>::type; using ConstIf = typename std::conditional<If, const T, T>::type;
...@@ -255,16 +253,11 @@ template <typename Fun, typename FunT = typename std::decay<Fun>::type> ...@@ -255,16 +253,11 @@ template <typename Fun, typename FunT = typename std::decay<Fun>::type>
using IsSmall = std::integral_constant< using IsSmall = std::integral_constant<
bool, bool,
(sizeof(FunT) <= sizeof(Data::small) && (sizeof(FunT) <= sizeof(Data::small) &&
#if defined(__GNUC__) && !defined(__clang__)
// GCC has a name mangling bug that causes hard errors if we use noexcept
// directly here. Last tested at gcc 5.3.0.
// See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70790
std::is_nothrow_move_constructible<FunT>::value
#else
// Same as is_nothrow_move_constructible, but w/ no template instantiation. // Same as is_nothrow_move_constructible, but w/ no template instantiation.
noexcept(FunT(std::declval<FunT&&>())) noexcept(FunT(std::declval<FunT&&>()))
#endif
)>; )>;
using SmallTag = std::true_type;
using HeapTag = std::false_type;
template <typename T> template <typename T>
bool isNullPtrFn(T* p) { bool isNullPtrFn(T* p) {
...@@ -282,6 +275,7 @@ ReturnType uninitCall(Data&, Args&&...) { ...@@ -282,6 +275,7 @@ ReturnType uninitCall(Data&, Args&&...) {
inline bool uninitNoop(Op, Data*, Data*) { inline bool uninitNoop(Op, Data*, Data*) {
return false; return false;
} }
} // namespace function } // namespace function
} // namespace detail } // namespace detail
...@@ -289,9 +283,13 @@ namespace impl { ...@@ -289,9 +283,13 @@ namespace impl {
template <typename ReturnType, typename... Args, bool Const> template <typename ReturnType, typename... Args, bool Const>
class Function<ReturnType(Args...), Const> final { class Function<ReturnType(Args...), Const> final {
// These utility types are defined outside of the template to reduce
// the number of instantiations, and then imported in the class
// namespace for convenience.
using Data = detail::function::Data; using Data = detail::function::Data;
using Op = detail::function::Op; using Op = detail::function::Op;
using Tag = detail::function::Tag; using SmallTag = detail::function::SmallTag;
using HeapTag = detail::function::HeapTag;
using Call = ReturnType (*)(Data&, Args&&...); using Call = ReturnType (*)(Data&, Args&&...);
using Exec = bool (*)(Op, Data*, Data*); using Exec = bool (*)(Op, Data*, Data*);
...@@ -308,13 +306,9 @@ class Function<ReturnType(Args...), Const> final { ...@@ -308,13 +306,9 @@ class Function<ReturnType(Args...), Const> final {
Function<ReturnType(Args...), false>&&) noexcept; Function<ReturnType(Args...), false>&&) noexcept;
friend class Function<ReturnType(Args...), !Const>; friend class Function<ReturnType(Args...), !Const>;
template <typename Fun, typename FunT = typename std::decay<Fun>::type> template <typename Fun>
Function( struct OpsSmall {
Fun&& fun, using FunT = typename std::decay<Fun>::type;
typename std::enable_if<IsSmall<Fun>::value, Tag>::
type) noexcept(noexcept(FunT(std::declval<Fun>())))
: Function() {
struct Ops {
static ReturnType call(Data& p, Args&&... args) { static ReturnType call(Data& p, Args&&... args) {
return static_cast<ReturnType>((*static_cast<ConstIf<FunT>*>( return static_cast<ReturnType>((*static_cast<ConstIf<FunT>*>(
(void*)&p.small))(static_cast<Args&&>(args)...)); (void*)&p.small))(static_cast<Args&&>(args)...));
...@@ -336,20 +330,23 @@ class Function<ReturnType(Args...), Const> final { ...@@ -336,20 +330,23 @@ class Function<ReturnType(Args...), Const> final {
return false; return false;
} }
}; };
template <typename Fun>
Function(Fun&& fun, SmallTag) noexcept : Function() {
using Ops = OpsSmall<Fun>;
if (!detail::function::isNullPtrFn(fun)) { if (!detail::function::isNullPtrFn(fun)) {
::new (&data_.small) FunT(static_cast<Fun&&>(fun)); ::new (&data_.small) typename Ops::FunT(static_cast<Fun&&>(fun));
exec_ = &Ops::exec; exec_ = &Ops::exec;
call_ = &Ops::call; call_ = &Ops::call;
} }
} }
template <typename Fun, typename FunT = typename std::decay<Fun>::type> template <typename Fun>
Function(Fun&& fun, typename std::enable_if<!IsSmall<Fun>::value, Tag>::type) struct OpsHeap {
: Function() { using FunT = typename std::decay<Fun>::type;
struct Ops {
static ReturnType call(Data& p, Args&&... args) { static ReturnType call(Data& p, Args&&... args) {
return static_cast<ReturnType>((*static_cast<ConstIf<FunT>*>(p.big))( return static_cast<ReturnType>(
static_cast<Args&&>(args)...)); (*static_cast<ConstIf<FunT>*>(p.big))(static_cast<Args&&>(args)...));
} }
static bool exec(Op o, Data* src, Data* dst) { static bool exec(Op o, Data* src, Data* dst) {
switch (o) { switch (o) {
...@@ -367,10 +364,15 @@ class Function<ReturnType(Args...), Const> final { ...@@ -367,10 +364,15 @@ class Function<ReturnType(Args...), Const> final {
return true; return true;
} }
}; };
data_.big = new FunT(static_cast<Fun&&>(fun));
template <typename Fun>
Function(Fun&& fun, HeapTag) : Function() {
using Ops = OpsHeap<Fun>;
data_.big = new typename Ops::FunT(static_cast<Fun&&>(fun));
call_ = &Ops::call; call_ = &Ops::call;
exec_ = &Ops::exec; exec_ = &Ops::exec;
} }
template <typename F, typename G = typename std::decay<F>::type> template <typename F, typename G = typename std::decay<F>::type>
using ResultOf = decltype(static_cast<ReturnType>( using ResultOf = decltype(static_cast<ReturnType>(
std::declval<ConstIf<G>&>()(std::declval<Args>()...))); std::declval<ConstIf<G>&>()(std::declval<Args>()...)));
...@@ -422,9 +424,8 @@ class Function<ReturnType(Args...), Const> final { ...@@ -422,9 +424,8 @@ class Function<ReturnType(Args...), Const> final {
* selected by overload resolution when `fun` is not a compatible function. * selected by overload resolution when `fun` is not a compatible function.
*/ */
template <class Fun, typename = ResultOf<Fun>> template <class Fun, typename = ResultOf<Fun>>
/* implicit */ Function(Fun&& fun) noexcept( /* implicit */ Function(Fun&& fun) noexcept(IsSmall<Fun>::value)
noexcept(Function(std::declval<Fun>(), Tag{}))) : Function(static_cast<Fun&&>(fun), IsSmall<Fun>{}) {}
: Function(static_cast<Fun&&>(fun), Tag{}) {}
/** /**
* For moving a `Function<X(Ys..) const>` into a `Function<X(Ys...)>`. * For moving a `Function<X(Ys..) const>` into a `Function<X(Ys...)>`.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment