modified jni logic(List-> Array)

[pluris]

In JNI code, it has been modified from List to Array.
If there are need to be fix, please comment.

Issue : #149

[jacobperron]

Thanks for looking into this 🙂

Do you have more specifics on where the performance impact is coming from?

From a usability perspective, it might be better to continue to use List at the higher layer (e.g. Node, AsyncParametersClient, etc) if the performance gains are strictly related to the changes to JNI code.

[pluris]

@jacobperron
In task, for example, I want to continuously transmit image data, but there was no problem in the Ros2 cpp application, but in Ros2Java, there was a serious performance degradation.
Because of this problem, I am using it by changing to Array.
I agree that List is better for usability. However, I am concerned about List performance. If you have any other ideas, please suggest.
If you don't want to change it, I'll close it.

[jacobperron]

To clarify my previous comment, I do think that switching to Array in the C code makes sense, but my question is if it is necessary to also change all of the API in the Java client code? For example, I wouldn't think that changing the types used by the parameters API would have a significant performance impact on publishing and subscribing to image topics.

I plan to investigate the performance issues myself and will give a more informed review later.

[jacobperron]

IIUC, the performance issue is coming from creating new Java objects for each element in the list.

[jacobperron]

I haven't found a way to improve performance while continuing to use the List type.
Maybe it is acceptable to completely switch to array types given the performance gains. This means that users will no longer be able to change the size of ROS message members with "list" type, for example:

sensor_msgs.msg.Image message = new sensor_msgs.msg.Image();

// Before
List<Byte> image_data = message.getData();
image_data.get(0);  // access
image_data.add((byte) 200);  // modify message

// After
byte[] image_data = message.getData();
image_data[0];  // access
// No easy way to modify message in place, must construct a new array
int[] new_image_data = Arrays.copyOf(image_data, image_data.length + 1);
new_image_data[arr.length] = (byte) 200;
message.setData(new_image_data);

But, perhaps not being able to change the size of a ROS message in-place is a good thing. It might avoid bugs where the user modifies the returned list and unintentionally modifies the member of the message.

@esteve Any thoughts on this? I think we really do need to address this performance issue.

[jacobperron]

I think a good option is to switch to arrays by default and offer a convenience method for getting List types. I've adapted this PR with additional changes and fixes: https://github.com/osrf/ros2_java/tree/jacob/list_to_array

I'll adapt it for dashing and open a PR when it's ready.

[jacobperron]

Re-applying this change in #165.