Skip to content

Clarify ConstSharedPtr achieves zero-copy - #7064

Open
MShields1986 wants to merge 4 commits into
ros2:lyricalfrom
MShields1986:lyrical
Open

Clarify ConstSharedPtr achieves zero-copy#7064
MShields1986 wants to merge 4 commits into
ros2:lyricalfrom
MShields1986:lyrical

Conversation

@MShields1986

Copy link
Copy Markdown

Description

The existing note on subscriber callback types incorrectly implied that no shared_ptr variant could achieve zero-copy, grouping mutable shared_ptr<T> and ConstSharedPtr together.
This is misleading: const std::shared_ptr<const T> & (ConstSharedPtr) uses immutable shared ownership and delivers zero-copy to multiple intra-process subscribers simultaneously, making it the correct pattern for one-to-many composition.

Changes:

  • Correct the shared_ptr sentence in the two-node pipeline section to distinguish ConstSharedPtr (zero-copy capable) from mutable shared_ptr<T> (not zero-copy)
  • Add a follow-on paragraph to the two-image-viewer section explaining that ConstSharedPtr callbacks avoid the fan-out copy penalty that UniquePtr incurs in one-to-many graphs

A more detailed write up with tests and findings is available here

Did you use Generative AI?

No

Additional Information

The correction to the shared_ptr note is a factual fix as the previous wording could lead readers to incorrectly avoid ConstSharedPtr in one-to-many scenarios where it is the appropriate tool.

@MShields1986 MShields1986 changed the title docs(intra-process): clarify ConstSharedPtr achieves zero-copy for one-to-many Clarify ConstSharedPtr achieves zero-copy Aug 13, 2026
@MShields1986
MShields1986 marked this pull request as ready for review August 13, 2026 10:31
Comment thread source/Tutorials/Demos/Intra-Process-Communication.rst Outdated
Re-add the plain const T & callback type to the list of subscriber
signatures that do not achieve zero-copy, addressing reviewer feedback
that this case was dropped from the previous clarification.

Co-Authored-By: MShields1986 <matthew.shields@dexory.com>
Comment thread source/Tutorials/Demos/Intra-Process-Communication.rst Outdated
Comment thread source/Tutorials/Demos/Intra-Process-Communication.rst Outdated
This is because we're publishing and subscribing with ``std::unique_ptr``\ s which allow ownership of a message to be moved around the system safely.
You can also publish and subscribe with ``const &`` and ``std::shared_ptr``, but zero-copy will not occur in that case.
You can also subscribe using a ``const std::shared_ptr<const T> &`` (``ConstSharedPtr``) callback, which shares immutable ownership of the message and achieves zero-copy even with multiple subscribers, so long as a ``std::unique_ptr`` was passed to the publisher.
Subscribing with a plain ``const T &`` or a mutable ``std::shared_ptr<T>`` will not achieve zero-copy.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if you still see this with a newer version of rclcpp (Lyrical/Rolling). IIRC, const T & should now have the same effect as const std::shared_ptr<const T> or std::shared_ptr<const T>.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see exactly what I tested in more detail here. I could add cases but would be good to get a full matrix of what anyone wants to see first.

Might be scope creep for this specific PR though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coincidentally, I actually wrote a similar Discourse post at the beginning of this year documenting these behaviors (though mine is not as comprehensive as yours). That post eventually led to a PR fixing the const T & case, so I think this sentence may no longer be accurate.

My two cents would be to validate it and update the sentence if needed, or simply remove it since the tutorial doesn't mention it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants