Reduce GID storage to 16 bytes. #345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
That is, the size of an RMW GID should be defined by the RMW layer and the RMW implementations should conform to it. With that said, 16 bytes is plenty (and happens to match the size of a GUID in DDS, which is also 16 bytes). So reduce this to 16 bytes. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
clalancette
requested review from
methylDragon,
ivanpauno and
wjwwood
as code owners
|
Full CI:
|
mjcarroll
approved these changes
Jan 23, 2023
|
This is approved, and CI is clean. I'm going to go ahead and merge this (and the corresponding rmw_dds_common PR). Thanks for the review. |
delete-merged-branch
bot
deleted the
clalancette/reduce-gid-storage-to16bytes
branch
christophebedard
added a commit
to ros2/ros2_tracing
that referenced
this pull request
Oct 7, 2024
The size changed from 24 to 16 bytes in ros2/rmw#345. The same size is defined in `tracetools`, and it was never updated. In practice, reading 24-16=8 random extra bytes didn't change much, since nothing is currently relying on the GID values, at least not relying on getting the same GID for the same object from two different systems, because it doesn't work with `rmw_cyclonedds`, see ros2/rmw_cyclonedds#377. Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
christophebedard
added a commit
to ros2/ros2_tracing
that referenced
this pull request
Oct 8, 2024
The size changed from 24 to 16 bytes in ros2/rmw#345. The same size is defined in `tracetools`, and it was never updated. In practice, reading 24-16=8 random extra bytes didn't change much, since nothing is currently relying on the GID values, at least not relying on getting the same GID for the same object from two different systems, because it doesn't work with `rmw_cyclonedds`, see ros2/rmw_cyclonedds#377. Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
That is, the size of an RMW GID should be defined by the RMW
layer and the RMW implementations should conform to it. With
that said, 16 bytes is plenty (and happens to match the size
of a GUID in DDS, which is also 16 bytes). So reduce this
to 16 bytes.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Note that this is a rehash of #328, but I think we have sufficient evidence now that this always should have been 16 bytes. This will also require a PR in rmw_dds_common, which is coming up next.