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.

Sign up for GitHub

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

Jump to bottom

Add in a more helpful usage message to allocator_tutorial. #409

Merged
merged 1 commit into from
Nov 15, 2019
Merged

Add in a more helpful usage message to allocator_tutorial. #409

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions demo_nodes_cpp/src/topics/allocator_tutorial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ int main(int argc, char ** argv)
std::list<std::string> keys = {"intra", "intraprocess", "intra-process", "intra_process"};
bool intra_process = false;

printf("This simple demo shows off a custom memory allocator to count all\n"
"instances of new/delete in the program. It can be run in either regular\n"
"mode (no arguments), or in intra-process mode (by passing 'intra' as a\n"
"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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clalancette why not turning this into a usage message, only shown if --help or -h are present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thus, I don't think we can say too much about the allocations.

Related ticket: #292


if (argc > 1) {
for (auto & key : keys) {
if (std::string(argv[1]) == key) {
Expand Down