Rationale about what to test for performance

[Blast545]

Took some time to check the rcutils submodules, here's a list of them and what I consider that should be tested for each of those in terms of performance:

├── allocator.h
I/O, allocation, runtime critical

• Allocation time
• Memory leaks
• Asymptotic complexity

├── cmdline_parser.h
STABLE, not often used

• Not required

├── env.h
STABLE, I/O

• Not required, maybe check platform runtime differences

├── error_handling.h
STABLE, I/O, Allocation, runtime critical

• Allocation time
• Memory leaks
• Asymptotic complexity

├── filesystem.h
STABLE, I/O

• Not required, maybe check platform runtime differences

├── find.h
STABLE, Algorithm

• Asymptotic complexity

├── format_string.h
Allocation, Critical runtime

• IMO This could be tested through logging or a higher level module

├── get_env.h
STABLE, I/O, conf code

• Not required

├── isalnum_no_locale.h
STABLE

• Not required

├── logging.h
Allocation, Critical runtime

• Allocation time
• Memory leaks
• Asymptotic complexity

├── process.h
STABLE, IO, Allocation, not often used

• Not required

├── qsort.h
Algorithm

• Asymptotic complexity

├── repl_str.h
STABLE, Algorithm

• Asymptotic complexity

├── shared_library.h
IO, Conf code or critical runtime (?

• Allocation time
• Memory leaks
• Asymptotic complexity

├── snprintf.h
STABLE, IO

• IMO This could be tested through logging or a higher level module. Or simple tests to check platform differences

├── split.h
STABLE, Algorithm

• Asymptotic complexity

├── stdatomic_helper.h
STABLE, IO, Critical runtime

• Can those be tested for performance?

├── strdup.h
Allocation

• Memory leaks

├── strerror.h
STABLE, IO, Critical runtime

• Allocation time
• Memory leaks
• Asymptotic complexity

├── time.h
IO

• Runtime platform differences, maybe test through higher level modules

Custom defined primitives:
│   ├── array_list.h
│   ├── char_array.h
│   ├── hash_map.h
│   ├── string_array.h
│   ├── string_map.h
│   └── uint8_array.h

• Test basic operations runtime performance + Asymptotic complexity
• Memory leaks

[Blast545]

As part of this analysis I've come up with a list of "categories" that could be used to "tag" each module/package under discussion if requires performance testing or not. This way, for some specific defined categories we can determine the desired tests and speed up the decision for other packages.

Some proposed categories:

• Stable: Code that isn't likely to be changed. Wraps C++ functionality or system calls.

• I/O: Functions accessing OS resources or making system calls.

• Algorithm: Sorting/Searching/Accessing algorithms

• Critical runtime: Functions commonly used in "production code"

• Custom primitive type: lists, hashs, arrays and other primitives that can be defined

• RMW dependant: Code that interfaces DDS. Those could be tested using mocks and RMW sections tested under integration tests.

• Thread safety concern: Code that requires to be thread safe. Although I think this goes out of scope of the performance tests, not sure.

• Configuration code: Code that's used in production environment, but used only once or twice for configuration of the system.

• Not often used: As the category says, code developed probable to show specific functionality or showing a demo.

[Blast545]

Please take a look and let me know if this rationale makes sense and how it can be improved to start writing the required tests for rcutils

[hidmic]

Based on above's categorization, and with a priori knowledge of how these APIs are often used, I'd start out with:

• logging.h
• error_handling.h

I'd also include string_map.h, as the logging.h module uses it heavily (which IMHO makes logging unnecessarily expensive, but that's a separate discussion).

---

Aside from that, a few minor comments:

here's a list of them and what I consider that should be tested for each of those in terms of performance:

Perhaps presenting this as a Markdown table may ease reading.

RMW dependant: Code that interfaces DDS.

I'd rather not mention DDS here. RMWs are not necessarily DDS based.

Not often used: As the category says, code developed probable to show specific functionality or showing a demo.

Not sure what you mean here?

[wjwwood]

Some proposed categories:

These categories seem nice (with @hidmic's concerns addressed of course), I'm adopting them somewhat in my work on rclcpp.

[hidmic]

@Blast545 @chapulina does this PR and #288 still make sense?

[chapulina]

Even though we've already satisfied the requirements for quality level 1, more testing can't hurt. It looks like #288 is close to getting in, so I think it would be a shame to drop it.

[Blast545]

@chapulina @hidmic I just pushed in the branch to address the remaining comments, I'll try to push to merge that. Besides that, I think this issue can be closed.