WG21-SG14/SG14

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 to memcpy 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!