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.

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

Added benchmark test for rosidl_generator_c #529

Closed

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Sep 15, 2020

This PR builds on top of these other PRs

Addes some performance test to the interfaces init() and destroy() functions.

Signed-off-by: ahcorde ahcorde@gmail.com

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added the tests label Sep 15, 2020
@ahcorde ahcorde self-assigned this Sep 15, 2020
@ahcorde ahcorde force-pushed the ahcorde/benchmark_rosidl_generator_c branch from c8d56f3 to 2d45222 Compare September 22, 2020 08:56
Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

I don't understand why the tests are exercising array assignments. We're not really expecting C operations like arr->bool_values[i] = true; to change in performance, and it doesn't result in any lines of code in rosidl_generator_c being exercised, so I don't see any reason to track it here. rosidl_runtime_c__String__assign is already tracked by another test, so I don't see any reason to measure that performance either.

After looking at the generated code output from this package, I really only see allocation and de-allocation functions that could be worth testing. Am I missing something?

@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.5)

project(rosidl_generator_c C)
project(rosidl_generator_c C CXX)
Copy link
Member

Choose a reason for hiding this comment

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

Rather than this, enable the language only if BUILD_TESTING is specified as is done here:

# For gtest
enable_language(CXX)
# Default to C++14
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 14)
endif()

Suggested change
project(rosidl_generator_c C CXX)
project(rosidl_generator_c C)

@ahcorde ahcorde closed this Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants