Add in a more helpful usage message to allocator_tutorial. #409
Conversation
| "command-line argument)'. It will then publish a message to the\n" | ||
| "'/allocator_tutorial' topic every 10 milliseconds until Ctrl-C is pressed.\n" | ||
| "At that time it will print a count of the number of allocations and\n" | ||
| "deallocations that happened during the program.\n\n"); |
There was a problem hiding this comment.
@clalancette why not turning this into a usage message, only shown if --help or -h are present?
There was a problem hiding this comment.
The main reason I wanted to print this at startup time is that running the binary is quite silent:
ros2 run demo_nodes_cpp allocator_tutorial
(it only prints a single debugging message). Unless you go look at the code, it isn't clear at all what this tutorial is supposed to do, or what users should expect to happen. So I thought that unconditionally printing this message would help future testers not ask the same question.
There was a problem hiding this comment.
Could we maybe say what the expected results would be? Like if you are using intra you would expect allocations to be lower?
Since this is just a demo, I feel like erring on the side of more verbosity is quite alright.
There was a problem hiding this comment.
I just tested out the difference between running with 'intra' and without. I ran the tutorial for 10 seconds, using the following command:
timelimit -t 10 -s 2 install/demo_nodes_cpp/lib/demo_nodes_cpp/allocator_tutorial
This causes the tutorial to generate on average 4089 calls to global new and 13522 calls to allocator new.
If I instead run:
timelimit -t 10 -s 2 install/demo_nodes_cpp/lib/demo_nodes_cpp/allocator_tutorial intra
This cause the tutorial to generate on average 3910 calls to global new and 13588 calls to allocator new. So with intra-process there are fewer calls to the global one, but there are more calls to the allocator one. Also, the difference is probably in the noise, since I wasn't on a "quiet" system.
Thus, I don't think we can say too much about the allocations. I'd say we can probably go forward with this as-is.
I'll run CI on it just to make sure I didn't mess something obvious up.
There was a problem hiding this comment.
Thus, I don't think we can say too much about the allocations.
Related ticket: #292
Signed-off-by: Chris Lalancette <[email protected]>
clalancette
force-pushed
the
allocator-tutorial-language
branch
from
815c5fd
to
a3b407e
Compare
|
CI:
|
|
@mjcarroll This is ready to go; I leave it to you whether you want to merge this for Eloquent or wait. |
Signed-off-by: Chris Lalancette [email protected]