inplace_function move constructor should be noexcept
Quuxplusone opened this issue · 6 comments
Right now, the following program performs poorly:
https://wandbox.org/permlink/vLEl9Owmp1exrkKA
struct Widget {
Widget() {}
Widget(Widget&&) noexcept { puts("move"); }
Widget(const Widget&) noexcept { puts("copy"); }
};
auto f = [w = Widget()]() {};
using IPF = stdext::inplace_function<void(), 32>;
std::vector<IPF> v;
v.emplace_back(f);
v.emplace_back(f);
[...]
The vector resize does copy
instead of move
, because inplace_function
's move constructor is not marked noexcept
, and vector is pessimized in that case.
I'd like to see us explicitly drop support for non-noexcept-moveable types, preferably by specifying that the move constructor will call std::terminate
if the wrapped object throws during its move-construction. Then the patch to de-pessimize the vector code above would be a single keyword: just add noexcept
to the move constructor. (And to swap
; and optionally to move-assignment.)
@carlcook thoughts?
Considering that inplace_function
is most likely to be used in environments where exceptions are not desirable/not used anyway due to performance or code-size reasons, I believe that making the move constructor/move assignment operator noexcept is perfectly reasonable.
Even more important, inplace_function
's design makes it useful to store in vectors and other contiguous containers. Since the move constructor isn't noexcept
, it means a growing vector cannot use it to move elements. The vector will use the copy constructor.
Of course making inplace_function
trivially copyable would be better, allowing vectors to memcpy
when growing, and trivially destructible to make release practically no cost.
Of course making
inplace_function
trivially copyable would be better, allowing vectors tomemcpy
when growing, and trivially destructible to make release practically no cost.
That sounds like a new type to me: inplace_trivial_function
. Being trivial, such a type would not be able to "own" any resources, e.g. it could hold [s = "foo"]{ return std::string(s); }
but not [s = std::string("foo")]{ return s; }
.
I should also point out that perhaps a more useful constraint for a new type would be inplace_relocatable_function
, which would be allowed to own any resource that itself was trivially relocatable. Such a type could hold [s = std::string("foo")]{ return s; }
and yet also permit vector
to use memcpy
for reallocation.
However, depending on the use-case, you might be able to get away with using @SuperV1234's function_ref
(coming to C++2a Real Soon Now, I think!), which is non-owning and therefore trivially copyable.
@Quuxplusone Thank you for the clarification. trivially_relocatable
looks very (very) promising! In the end, fast iteration is more important than growth speed, so I wouldn't say it is a crucial feature. More of a "nice to have".
On another note, since ultimately the inplace_function
data is a stack array, moving vs. copying wouldn't give any perf gain during growth I believe? I guess we'll have to wait for trivially_relocatable.
trivially_relocatable
looks very (very) promising!
Thank you! My P1144 "Object relocation in terms of move plus destroy" will be in the pre-San Diego mailing next month, but I will not be attending San Diego in person, so it's relying on word of mouth and the kindness of strangers to see whether anything can be made to happen for C++2a.
On another note, since ultimately the
inplace_function
data is a stack array, moving vs. copying wouldn't give any perf gain during growth I believe?
I believe you believe wrong about that. You'd be right if we were talking about inplace_trivial_function
, where moving and copying would both be trivial and thus both equivalent to memcpy
; but in the case of SG14 inplace_function
, both moving and copying are type-erased operations. So for example
inplace_function<std::string()> a = [s = std::string("too long for the Small String Optimization")]() { return s; };
auto b = a; // performs a *copy* of the wrapped lambda, which copies `s`
auto c = std::move(a); // performs a *move* of the wrapped lambda, which moves-out-of `s`
(Right now c = std::move(a)
performs a move. Patch #129 would make it perform a relocate instead, which would make it even more efficient, and as a bonus, would leave a
in the disengaged state instead of an unspecified-but-valid-engaged state. I should probably just merge #129; it's been sitting long enough.)
b = a
has to do a heap allocation to make a second copy of the data in s
.
We expect c = std::move(a)
to be more efficient, because it can just transfer ownership of the data in s
.
#believedwrong :D
Thanks so much for helping me wrap my head around this stuff. Keep up the great work, sg14 proposals (and headers) are awesome and give me a lot of hope for the future of c++ in high-perf applications.
Have a great day!