Feature/image performance #128
Open
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
Named {member}_Count as this is the normal .NET lingo.
removed getsize_array_field c function as well as it is constant,
no need for interop
Idl arrays now map as System.Array instead of List<T>. This is the more natural C# type for them.
- size gets checked on writing to the native handle of the message. - change foreach to for to avoid multi threaded race conditions while writing to native memory. (the upper bound is now local and cannot be changed from somewhere else)
`List<T>.Count` is of c# type `int`, wich is 32 bit signed. C does use `sizeof_t` wich is 32/64 bit dependent. So this does marshal at least correctly, ignoring cast from 64 bit to 32 bit and vice versa for now. getting marshaling/conversations 100% correct is anoher issue for another day.
Most importent the `IntPtr Handle` property from `ROS2.Interfaces.IDisposable` should not be public. `ROS2.Interfaces.IDisposable` was missleading as well. (`System.IDisposable` is the well kown one) Made appropriate memers internal as well and added some sealed and static modifiers. This was done in preperation for adding SafeHandles to avoid memory leaks, as this would change the implementation detail that was made public by the interfaces. Abstractions such as interfaces have to be designed for extensability. See https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/abstractions-abstract-types-and-interfaces
As of before this commit native recources for rcl_node_t, rcl_client_t, rcl_publisher_t, rcl_service_t and rcl_subscription_t didn't get released. Used a trick from dotnet/runtime to handle the requirement that node has to be released last. Also added some TODOs to check return values and for checks near allocations use the new throw helper.
- Refactored and renamed some methods to be more consistent. - Added some error handling. - Don't try to take things if the wait set did return a timeout.
- renamed IMessage to IRosMessage - moved interfaces for generated types to ROS2 namespace - renamed interface members - used `global::*` in some places to avoid name colisions - added editor browsable never to the members of the interfaces that schould not be used outside of rcldotnet
- Avoids doing reflection over and over. - If we can require static abstract interface members this could go away.
- move RCLExeptionHelper from rcldotnet_common to rcldotnet for acces to rcl_get_error_string() - get error string using rcl_get_error_string() and add this to the exception message - change throw helper to avoid unrachable code - also some other cleanup
error CS0136: A local or parameter named 'request' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter
This allows them to be used in switch cases and other places.
The field access after Interlocked.CompareExchange might not reflect the updated value. Or does it? Anyways this makes it more clear.
This does not change the C# side of the generated messages to avoid even more complexity in the templates. As bools in C# pInvokes get marshalled as 32 bit integer by default there is no strictly need to change the code for bool fields.
Most of the documentation was taken from ros2_rust. https://github.com/ros2-rust/ros2_rust/blob/main/rclrs/src/qos.rs
|
In case anyone is interested, I actually tackled the problem of large, basic message data (including images) taking long times to transfer across the interop and causing garbage collection issues by updating the message generation code to block copy array and list data where possible. I also used reflection to access the array backed lists in order to minimise garbage collection. These features are currently on the dev branch of my fork and will be added to a PR when my existing one has been merged. |
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.
No description provided.