facontidavide / ros_type_introspection

Deserialize ROS messages that are unknown at compilation time
MIT License
61 stars 30 forks source link

Building a tree with values from FlatMessage #33

Closed StefanFabian closed 5 years ago

StefanFabian commented 5 years ago

Hey,

first off, thank you for working on this! However, I ran into a problem. I'm trying to build a tree from the deserialized message in FlatMessage. I assumed the tree member would be a perfect fit and it is partly. I can build my tree by traversing it but I don't see how I can retrieve the values from the leaves. The values are stored as pairs of StringTreeLeaf and the value in FlatMessage. But how do I obtain the StringTreeLeaf representing the StringTreeNode. It looks like I have to go through the values vector for each leaf node to obtain the value which is far from optimal. To be honest, I don't really see the point of the StringTreeLeaf struct. For example, why does it have a fixed size index array? Is there a limit for ros messages that they can't be more than 8 layers deep? Why isn't the mapping in FlatMessage just from StringTreeNode's that are leaves to values? And why is it a vector of pairs instead of a map?

Best, Stefan

facontidavide commented 5 years ago

The library is designed with speed in mind, sacrificing partially the usability.

To be honest, I don't really see the point of the StringTreeLeaf struct.

From the point of view of the user, StringTreeLeaf is a pain in the ass, but from MY point of view, it makes applying RenamingRules much faster.

For example, why does it have a fixed size index array?

The fixed size is to improve avoid memory allocation. this improve dramatically the speed.

Is there a limit for ros messages that they can't be more than 8 layers deep?

8 is the maximum number of array indexes, not the number of total layers of the message. Therefore, it is a very conservative number.

Why isn't the mapping in FlatMessage just from StringTreeNode's that are leaves to values?

You are not understanding the way the StringTreeNode works ;) When the tree contains an array at some point of the type hierarchy, it use a placeholder for the index. The actual number with the correct index is retrieved from StringTreeLeaf::index_array.

And why is it a vector of pairs instead of a map?

Brutal speed improvement :)

EDIT:

Please thrust that I know what I am doing. Anything that may look weird to you is the result of multiple iterations to make the code as fast as possible. I used benchmarking and profiling to find as many hot spots as possible. May be it can be improved even further, but believe me when I say that I do know the difference between std::map and a vector of std::pair ...

facontidavide commented 5 years ago

It looks like I have to go through the values vector for each leaf node to obtain the value which is far from optimal.

What would be for you an optimal way to do it? I will be happy to receive Pull Request if you think that we can improve efficiency even further :+1:

facontidavide commented 5 years ago

To understand the relationship between StringTreeLeaf and the tree, you should probably take a loop to this method https://github.com/facontidavide/ros_type_introspection/blob/840748d21f5d2135a3a80c1baa2b68b63c0ce95d/src/stringtree_leaf.cpp#L42

In short:

Done!

StefanFabian commented 5 years ago

Thank you for your quick and detailed replies. I've already implemented it based on your first comment. I did it similarly to how you described. Looping over the values / names vector, recursively going up until the root and then back down to the leaf to create the structure and insert the value. It works like a charm!