-
Notifications
You must be signed in to change notification settings - Fork 196
Clarify publish behavior for shared_ptr and unique_ptr #344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: gh-pages
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -344,6 +344,11 @@ A copy of the message will be given to all the `Subscription`s requesting owners | |
| The following tables show a recap of when the proposed implementation has to create a new copy of a message. | ||
| The notation `@` indicates a memory address where the message is stored, different memory addresses correspond to different copies of the message. | ||
|
|
||
| **Note on current behavior**: The actual implementation behavior differs from the proposed implementation as described in [rclcpp issue #2872](https://github.com/ros2/rclcpp/issues/2872): | ||
| - **shared_ptr messages**: Cannot be published. | ||
| - **unique_ptr messages**: When using `std::move()`, zero-copy transfer is achieved. | ||
| - **raw messages**: Both `msg` and `std::move(msg)` result in copying the message. | ||
|
|
||
| #### Publishing UniquePtr | ||
|
|
||
|
|
||
|
|
@@ -352,22 +357,10 @@ The notation `@` indicates a memory address where the message is stored, differe | |
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> | @1 | | ||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> unique_ptr\<MsgT\> | @1 <br> @2 | | ||
| | unique_ptr\<MsgT\> @1 | shared_ptr\<MsgT\> | @1 | | ||
| | unique_ptr\<MsgT\> @1 | shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @1 | | ||
| | unique_ptr\<MsgT\> @1 | shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. your change here is wrong
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your suggestions. I just took a deeper look into the code, and I noticed that only I think the current implementation makes sense, as non-const sharedptr could be changed and an extra copy is necessary for safety reasons. But the table doesn't show the constness of MsgT. It also seems weird that the first part of design documentation specifies |
||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 | | ||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 <br> @2| | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think these should be removed |
||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 <br> @3 <br> @3| | ||
|
|
||
| #### Publishing SharedPtr | ||
|
|
||
| | publish\<T\> | BufferT | Results | | ||
| | ----------------------- | ----------------------- | ----------------------- | | ||
| | shared_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> | @2 | | ||
| | shared_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> unique_ptr\<MsgT\> | @2 <br> @3 | | ||
| | shared_ptr\<MsgT\> @1 | shared_ptr\<MsgT\> | @1 | | ||
| | shared_ptr\<MsgT\> @1 | shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @1 | | ||
| | shared_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @2 <br> @1 | | ||
| | shared_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @2 <br> @1 <br> @1| | ||
| | shared_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @2 <br> @3 <br> @1 <br> @1| | ||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 <br> @3| | ||
| | unique_ptr\<MsgT\> @1 | unique_ptr\<MsgT\> <br> unique_ptr\<MsgT\> <br> shared_ptr\<MsgT\> <br> shared_ptr\<MsgT\> | @1 <br> @2 <br> @3 <br> @4| | ||
|
|
||
| The possibility of setting the data-type stored in each buffer becomes helpful when dealing with more particular scenarios. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this section.
The fact that you can't publish shared ptr is not necessarily related to this design.
Moreover, i don't really see many references to publishing shared ptrs in this page.
these should be removed