Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new message bindings that uses rosidl_generator_c #774

Open
wants to merge 68 commits into
base: develop
Choose a base branch
from

Conversation

koonpeng
Copy link
Collaborator

This introduces is a new message binding system that uses native c code from rosidl_generator_c that I have been experimenting with. As this is a pretty big change to the core systems, it is turned off by default. Right now it's working for our use cases which most consist of pub/sub and service clients, I also managed to get it working on node v14 as it does not use ref.

Below description copied from the README gives a more detailed explaination on the new bindings.


In order to use the new bindings, you must either:

set RCLNODEJS_USE_ROSIDL=1 environment variable.
or
add a .npmrc file in your project directory with rclnodejs_use_rosidl=true.

The new experimental message bindings uses ros interfaces to perform serialization. The main advantage of the new bindings is better performance, it is ~25% faster for large messages (1mb) and ~800% faster for small messages (1kb). It is also safer as memory is managed by v8, you will no longer get undefined behaviors when you try to reference a message outside the subscription callbacks. Also as a result of moving to v8 managed memory, it fixes some memory leaks observed in the current bindings.

The downside is that the new bindings is not API compatible, it does a number of things differently.

  1. The new bindings initialize nested message as plain js objects instead of the wrappers classes. As a result, they don't contain wrapper methods, for example, this wouldn't work
const msg = new UInt8MultiArray();
console.log(msg.hasMember('foo')); // ok, `msg` is a UInt8MultiArrayWrapper
console.log(msg.layout.hasMember('bar')); // error, `layout` is a plain js object, there is no `hasMember` method
  1. There is no array wrappers.
const UInt8MultiArray = rclnodejs.require('std_msgs').msg.UInt8MultiArray;
const Byte = rclnodejs.require('std_msgs').msg.Byte;
const byteArray = new Byte.ArrayType(10); // error, there is no `ArrayType`
  1. Primitives are initialized to their zero value.
const Header = rclnodejs.require('std_msgs').msg.Header;
let header = new Header();
console.log(typeof header.frame_id); // 'string', in the old bindings this would be 'undefined'
  1. Shortform for std_msg wrappers are not supported.
const String = rclnodejs.require('std_msgs').msg.String;
const publisher = node.createPublisher(String, 'topic');
publisher.publish({ data: 'hello' }); // ok
publisher.publish('hello'); // error, shortform not supported
  1. Primitive arrays are always deserialized to typed arrays.
const subscription = node.createSubscription(
  'std_msgs/msg/UInt8MultiArray',
  'topic',
  (msg) => {
    console.log(msg.data instanceof Uint8Array); // always true, even if typed array is disabled in rclnodejs initialization
  }
);
  1. No conversion is done until serialization time.
const UInt8MultiArray = rclnodejs.require('std_msgs').msg.UInt8MultiArray;
const msg = new UInt8MultiArray();
msg.data = [1, 2, 3];
console.log(msg.data instanceof Uint8Array); // false, assigning `msg.data` does not automatically convert it to typed array.
  1. Does not throw on wrong types, they are silently converted to their zero value instead.
const String = rclnodejs.require('std_msgs').msg.String;
const publisher = node.createPublisher(String, 'topic');
publish.publish({ data: 123 }); // does not throw, data is silently converted to an empty string.
  1. Message memory is managed by v8. There is no longer any need to manually destroy messages and
    do other house keeping.
// With the old bindings, this may result in use-after-free as messages may be deleted when they
// leave the callback, even if there are still references to it. You will have to deep copy the message
// and manually destroy it with its `destroy` method when you don't need it anymore.
//
// This is safe with the new bindings, `lastMessage` will either be `undefined` or the last message received.
// Old messages are automatically garbage collected by v8 as they are no longer reachable.
let lastMessage;
const subscription = node.createSubscription(
  'std_msgs/msg/UInt8MultiArray',
  'topic',
  (msg) => {
    lastMessage = msg;
  }
);
setTimeout(() => {
  console.log(lastMessage);
}, 1000);

@minggangw
Copy link
Member

Wow, that's really a BIG progress and optimization. ref is one of the most important dependency for rclnodejs, although it facilitate the development in the beginning, we have to depend on it heavily, and actually, we have met some problem that we cannot fix because of ref.

We can gain a lot if we can use c library directly, the performance improvement you mentioned is signification! I will read the code carefully in the following days, thanks!

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.

2 participants