osrf / ros2_serial_example

70 stars 15 forks source link

Get rid of problematic optimization for Transporter write() #40

Closed clalancette closed 5 years ago

clalancette commented 5 years ago

We inherited an optimization from PX4 where they attempt to avoid copies by having the caller of transporter.write() allocate the correct number of header bytes. However, this has a couple of different problems:

  1. It doesn't actually work if we use COBS, since there is some size amplification anyway
  2. It is a very non-intuitive interface for the caller

Because it isn't a huge bottleneck, just remove the optimization for now and allow the user to just pass their payload with write() taking care of adding a header (this involves some copies). If it turns out that this is really a bottleneck in the future, there are a couple of different ways that we could improve the performance here:

  1. The CRC16 calculation over the payload has to visit every byte, so we could think about combining crc16 calculation with a copy into the new buffer. This doesn't get rid of the allocation, but it does mean we only iterate over the bytes once.
  2. We could alleviate concern 1 above by having another method in Transporter() that returned the worst-case size needed. This would preserve the performance benefits while allowing it to work with COBS.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org