feat: Cache internal string 3x speedup - #265
Conversation
Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
``convert_to_py`` now allocates the Python message directly and writes to the ``_field`` slots instead of calling the class and assigning through the property setters. ``__init__`` used to recursively construct a default value for every field, all of which were immediately overwritten. ``convert_from_py`` reads the slots directly for the same reason. On a ``visualization_msgs/MarkerArray`` with 500 markers holding 200 nested elements each, ``convert_to_py`` went from 110 ms to 30 ms and ``convert_from_py`` from 22 ms to 9 ms. Behavior change: because the property setters are bypassed, ``ROS_PYTHON_CHECK_FIELDS=1`` no longer runs its type assertions when a message is converted from C to Python. It still applies to messages that user code constructs or assigns to. The values produced by the conversion are built by the generated C code itself and cannot have the wrong type. Assisted-by: Claude Opus 5
|
Pulls: #265
|
|
I retested this with a ping pong test of visualization markers. With this patches I get 60 msg/sec at 20% cpu load |
|
Tick the box to add this pull request to the merge queue (same as
|
wjwwood
left a comment
wjwwood
left a comment
There was a problem hiding this comment.
Neat, seems reasonable to me. May need more acceptance testing and/or review. I didn't dive too deep.
| PyObject * pymessage_class = PyObject_GetAttrString(pymessage_module, "@(message.structure.namespaced_type.name)"); | ||
| assert(pymessage_class); | ||
| if (pymessage_module == NULL) { | ||
| return NULL; |
There was a problem hiding this comment.
In the deleted code they were using asserts, and I don't know which is more appropriate here but I'd like to know if this results in a behavior change or not. I would think this would change how python handles the result.
There was a problem hiding this comment.
In release mode this would simply crash if pymessage_module is null.
In debug mode you would get an assert failed.
According to the AI return NULL will cause an exception later in the rclpy code.
I can reintroduce the asserts for debugging...
2 participants
Description
AI experiment to speed up serialization.
Fixes # (issue)
Is this user-facing behavior change?
No
Did you use Generative AI?
Yes, claude sonet
Additional Information
Draft for now