mqtt_cpp icon indicating copy to clipboard operation
mqtt_cpp copied to clipboard

Allow to store moveable payload when (aync_)publishing

Open ropieur opened this issue 5 years ago • 7 comments

Hi @redboltz, I read the issue #338 In your explanation, you mention that life_keeper is move captured, which is a good idea and lets suggest me that I could put a moveable payload (like msgpack::sbuffer) into life_keeper. The problem comes from std::any which requires copy constructible objects (its simply refuses moveable only objects) I could consider using ashared_ptr, but I find that this is unneeded heavy overhead (additional heap allocation, protected access by mutex, reference counting ...) and to me, allowing for moveable would be a good improvement.

ropieur avatar Sep 13 '19 07:09 ropieur

Do you have a suggestion for an alternative type other than std::any?

Originally std::any was a std::function, and I proposed replacing it with std::any to allow people to pass anything, including std::pair and std::tuple.

Also, is std::any only used in a strictly move-only fashion internally? I'm not entirely certain that it is.

Do you think it'll be relatively straight forward to implement a move-only std::any?

jonesmz avatar Sep 13 '19 07:09 jonesmz

Hi @ropieur ,

I noticed that interesting behavior.

See https://wandbox.org/permlink/9yxoG5EasqU9JZ2r

If my_shared_ptr's copy constructor is deleted, then compile error occurs. If my_shared_ptr's copy constructor is defined, no compile error occurs and the copy constructor is not called but move constructor is called.

That means that if you allocate your move only type as std::shared_ptr, performance penalty is only heap allocation. No atomic reference count increment/decrement happens.

In addition, you can wrap your move only type as follows: https://wandbox.org/permlink/z2PSk7Ymz7lzPpmR

This avoids heap allocation. But I think that it is too hackey...

redboltz avatar Sep 13 '19 07:09 redboltz

I don't know why std::any prohibits move only types. Maybe there is a deep reason. I'm not sure these kind of hack is safe or not. But I don't find any unsafe reason, so far.

redboltz avatar Sep 13 '19 08:09 redboltz

I made some research on the subject, but no convincing results. As asked by @jonesmz, I can't provide with alternative solution. Perhaps boost::any supports moveable, but unsure because the documentation sticks with the copy ctor constraint while implementation seems to provide with move semantics.

ropieur avatar Sep 13 '19 08:09 ropieur

After reading the code more carefully, the move semantics applies only to boost::any type, not the content.

ropieur avatar Sep 13 '19 08:09 ropieur

I noticed that the life_keeper is copied internally. See https://github.com/redboltz/mqtt_cpp/blob/69ee912408eedf9023840f7720a407acd867c66c/include/mqtt/endpoint.hpp#L12657

So we cannot use hacked_sbuffer approach. It causes assertion fail. We need to use shared_ptr but reference count operation happens only copy operation is happened. It is not frequently happens.

redboltz avatar Sep 13 '19 13:09 redboltz

See also https://stackoverflow.com/questions/57923561/move-only-type-adapting-stdany-with-dummy-copy-constructor-is-safe

redboltz avatar Sep 13 '19 13:09 redboltz