Move to abseil

[facontidavide]

Abseil, a C++ library built by Google, has been released a couple of days ago.
https://github.com/abseil/abseil-cpp
It has a lot of interesting features that will make the code of this library more expressive.

I am planning to build a catkinized version of abseil and depend on it

[facontidavide]

I am also realizing that may be, since more of the memory allocation is being avoided with the new API, it might make sense to remove the infamous SString in favour of string_view...

[ibtaylor]

Yeah, that sounds good. I was wondering, shouldn't you be able to preprocess/flatten the message definitions and the flat container just reference the single array of strings/fields from the definition?

I did some work on variant trying to reduce the string copies and stuff facontidavide:dev...ibtaylor:dev_ian, but it didn't really seem to improve anything after profiling (could have some good ideas though). Maybe it wasn't the copying of strings, but the number of them instead. Maybe you can clarify, for a uint8[1206], for instance, is that going to lead to 1206 strings and individual variants?

I don't know if I already mentioned, but it seems like variant might should be on individual builtins and vectors of builtins. All 'other' messages can be flattened into these as well. I was worried that maybe message definitions could be recursive in nature, but they cannot (it simply crashes the ros message code). Once flattened, then it seems like you just iterate through the fields reading what you need to instead of all the recursion and such.

[facontidavide]

I am not sure that I understoof your question, but I am trying to answer...

Yeah, that sounds good. I was wondering, shouldn't you be able to preprocess/flatten the message definitions and the flat container just reference the single array of strings/fields from the definition?

Since in the message there are vectors with variable length, it is not possible to preallocate the flatten message definition in advance. Things are even more complicated considering renaming.

Maybe it wasn't the copying of strings, but the number of them instead. Maybe you can clarify, for a uint8[1206], for instance, is that going to lead to 1206 strings and individual variants?

Indeed. This is due to the fact that I can not make asumptions about what the user wants. Large vectors of byte MIGHT be of course an exception, this is the reason why I reserved DeserializedMessage::blob to handle this special case (not implemented yet).

All 'other' messages can be flattened into these as well. I was worried that maybe message definitions could be recursive in nature, but they cannot (it simply crashes the ros message code). Once flattened, then it seems like you just iterate through the fields reading what you need to instead of all the recursion and such.

In my opinion messages ARE recursive in nature, but I rewrote the code to handle Builtin types directly to reduce the number or recursions.

I looked quickly at your code and I have few remarks:

1. Do not underestimate the fact that your variant is 4 bytes larges. it adds up quickly.

2. I am not copying or moving Variants with Strings so often, so you shouldn't notice much improvement in the refactoring.

3. There isn't any gain in handlying explicitly the size of flat_container->name and flat_container->value as you did in deserializeIntoFlatContainer. First because you are doing explicitly what vector does already under the hood, secondary because the "trick" to speed things up is to reuse the same instance of flat_container over and over again (memory is allocated only once the first time).

[facontidavide]

Ok, I eventually understood your reasoning about managing manually the size of flat_container->value and flat_container->name in deserializeIntoFlatContainer. And it is faster indeed but only on (drum roll please) the abseil version: https://github.com/facontidavide/ros_type_introspection/tree/dev_abseil

Benchmark on master: 5600 nsec
Benchmark on dev: 2000 nsec
Benchmark on dev_abseil: 1600 nsec

[facontidavide]

The catkinized abseil can be found here https://github.com/Eurecat/abseil-cpp

Summarizing of the changes:

• Removed SString and use std:.string instead to store new string and absl::string_view to pass reference to chunks of strings.

• Removed nonstd::VectorView in favor of absl::Span

• Applied the changes you did in deserializeIntoFlatContainer. The main advantage according to Valgrind is that I have 1 less memory allocation of std::string in flat_container->name.

[facontidavide]

I am quite happy with the integration of abseil. I am merging branch dev_abseil into dev

[ibtaylor]

In my opinion messages ARE recursive in nature, but I rewrote the code to handle Builtin types directly to reduce the number or recursions.

Can you explain? Afaik, a message Foo cannot contain a message Foo directly or indirectly.
If that is true, doesn't that mean we can flatten the messages instead of dealing with trees?

[facontidavide]

As you know, there are three steps:

1. parsing of the ROSMessage definition,
2. deserializing a message
3. move the data into a key/string vector, using if necessary the SubstitutionRules.

if I understood what you mean, you said that we can generate a flat description of the message during step 1).

Lets take for instance a geometry_msgs/Pose.
There is not problem unrolling this type into a flat sequence of key/value such as:

pose/position/x -> float64
pose/position/y -> float64
pose/position/z -> float64

pose/orientation/x -> float64
pose/orientation/y -> float64
pose/orientation/z -> float64
pose/orientation/w -> float64

So in this particular case you are right.

The problem is when we have a type that has a field with a vector of geometry_msgs/Pose. the size of this vector is known only during step 2. Even worse, this size is located in the middle of a message and we don't know the position of the 4 bytes to parse until we get there.

Unrolling a for loop will not be particularly beneficial, in my opinion. Using the lambda, which capture some variable by reference, already improved the speed of recursion.

[kartikmohta]

Just curious, any particular reason to use abseil over GSL?

[facontidavide]

Two reasons. The first is that gsl requires c++14, the second is that I believed I could use more features of abseil

[kartikmohta]

Thanks for the clarification! 🙂