[PATCH] D55045: Add a version of std::function that includes a few optimizations.

[Jordan Soyke via Phabricator]

jsoyke marked 3 inline comments as done.
jsoyke added inline comments.


================
Comment at: include/functional:1774
+template <typename _Tp>
+using __fast_forward =
+    typename _VSTD::conditional<_VSTD::is_scalar<_Tp>::value, _Tp, _Tp&&>::type;
----------------
sbenza wrote:
> jsoyke wrote:
> > This optimization wasn't in the last version, sbenza@ mentioned we might want to do it. It helps with the Invoke benchmarks, as much as 10-20%.
> > 
> > It's probably worth mentioning that this can  hurt performance in cases where the caller is using std::forward, or passing small value types by reference, but these are very uncommon (at least in my experience).
> Can you show how this can hurt?
> Do you have a benchmark that gets better when this is removed?
> 
> This will only trigger if the signature of the function itself takes by value.
> `std::function::operator()` will have it by value already, and the user callback will take it by value.
> If we pass by ref we will be forced to put it on the stack on a different address even if the caller passed an lvalue because `operator()`'s arguments forced a copy.
> 
> We will need to read that data and pass it in by value anyway. We are only moving that lvalue-to-rvalue decay before the vtable instead of in the vtable.
> 
> https://gcc.godbolt.org/z/R7JBq2
Never-mind, my reasoning was flawed. I was basing my experience on a piece of software that did something like fast_forward but internally dispatched to std::function. It had to move register objects to the stack and made the fast_forward code perform worse, but we're fixing that problem here.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55045/new/

https://reviews.llvm.org/D55045