From c0d72c3ee0ac806379b49ed10cf17364a4bd443a Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 18 Jul 2023 10:18:22 -0400 Subject: [PATCH] Stop using constref signature of benchmark DoNotOptimize. (#2238) * Stop using constref signature of benchmark DoNotOptimize. Newer versions of google benchmark (1.8.2 in my case) warn that the compiler may optimize away the DoNotOptimize calls when using the constref version. Get away from that here by explicitly *not* calling the constref version, casting where necessary. Signed-off-by: Chris Lalancette --- .../benchmark/benchmark_lifecycle_client.cpp | 11 ++++++---- .../benchmark/benchmark_lifecycle_node.cpp | 22 +++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_client.cpp b/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_client.cpp index c8d166ef66..59ed24489e 100644 --- a/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_client.cpp +++ b/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_client.cpp @@ -228,7 +228,8 @@ class BenchmarkLifecycleClient : public performance_test_fixture::PerformanceTes BENCHMARK_F(BenchmarkLifecycleClient, get_state)(benchmark::State & state) { for (auto _ : state) { (void)_; - const auto lifecycle_state = lifecycle_client->get_state(); + + lifecycle_msgs::msg::State lifecycle_state = lifecycle_client->get_state(); if (lifecycle_state.id != lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED) { const std::string msg = std::string("Expected state did not match actual: ") + @@ -268,7 +269,7 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_available_states)(benchmark::State & s for (auto _ : state) { (void)_; constexpr size_t expected_states = 11u; - const auto states = lifecycle_client->get_available_states(); + std::vector states = lifecycle_client->get_available_states(); if (states.size() != expected_states) { const std::string msg = std::string("Expected number of states did not match actual: ") + @@ -284,7 +285,8 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_available_transitions)(benchmark::Stat for (auto _ : state) { (void)_; constexpr size_t expected_transitions = 2u; - const auto transitions = lifecycle_client->get_available_transitions(); + std::vector transitions = + lifecycle_client->get_available_transitions(); if (transitions.size() != expected_transitions) { const std::string msg = std::string("Expected number of transitions did not match actual: ") + @@ -300,7 +302,8 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_transition_graph)(benchmark::State & s for (auto _ : state) { (void)_; constexpr size_t expected_transitions = 25u; - const auto transitions = lifecycle_client->get_transition_graph(); + std::vector transitions = + lifecycle_client->get_transition_graph(); if (transitions.size() != expected_transitions) { const std::string msg = std::string("Expected number of transitions did not match actual: ") + diff --git a/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_node.cpp b/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_node.cpp index 0bf3c48ab9..4be9ad13af 100644 --- a/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_node.cpp +++ b/rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_node.cpp @@ -97,13 +97,15 @@ class BenchmarkLifecycleNode : public performance_test_fixture::PerformanceTest BENCHMARK_F(BenchmarkLifecycleNode, get_current_state)(benchmark::State & state) { for (auto _ : state) { (void)_; - const auto & lifecycle_state = node->get_current_state(); + const rclcpp_lifecycle::State & lifecycle_state = node->get_current_state(); if (lifecycle_state.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED) { const std::string message = std::string("Node's current state is: ") + std::to_string(lifecycle_state.id()); state.SkipWithError(message.c_str()); } - benchmark::DoNotOptimize(lifecycle_state); + // Google benchmark 1.8.2 warns that the constref DoNotOptimize signature may be optimized away + // by the compiler. Cast const away to ensure we don't get that problem (and warning). + benchmark::DoNotOptimize(const_cast(lifecycle_state)); benchmark::ClobberMemory(); } } @@ -112,7 +114,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_available_states)(benchmark::State & sta for (auto _ : state) { (void)_; constexpr size_t expected_states = 11u; - const auto lifecycle_states = node->get_available_states(); + std::vector lifecycle_states = node->get_available_states(); if (lifecycle_states.size() != expected_states) { const std::string msg = std::to_string(lifecycle_states.size()); state.SkipWithError(msg.c_str()); @@ -126,7 +128,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_available_transitions)(benchmark::State for (auto _ : state) { (void)_; constexpr size_t expected_transitions = 2u; - const auto & transitions = node->get_available_transitions(); + std::vector transitions = node->get_available_transitions(); if (transitions.size() != expected_transitions) { const std::string msg = std::to_string(transitions.size()); state.SkipWithError(msg.c_str()); @@ -140,7 +142,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_transition_graph)(benchmark::State & sta for (auto _ : state) { (void)_; constexpr size_t expected_transitions = 25u; - const auto & transitions = node->get_transition_graph(); + std::vector transitions = node->get_transition_graph(); if (transitions.size() != expected_transitions) { const std::string msg = std::string("Expected number of transitions did not match actual: ") + @@ -162,18 +164,20 @@ BENCHMARK_F(BenchmarkLifecycleNode, transition_valid_state)(benchmark::State & s reset_heap_counters(); for (auto _ : state) { (void)_; - const auto & active = + const rclcpp_lifecycle::State & active = node->trigger_transition(lifecycle_msgs::msg::Transition::TRANSITION_ACTIVATE); if (active.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE) { state.SkipWithError("Transition to active state failed"); } - const auto & inactive = + const rclcpp_lifecycle::State & inactive = node->trigger_transition(lifecycle_msgs::msg::Transition::TRANSITION_DEACTIVATE); if (inactive.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE) { state.SkipWithError("Transition to inactive state failed"); } - benchmark::DoNotOptimize(active); - benchmark::DoNotOptimize(inactive); + // Google benchmark 1.8.2 warns that the constref DoNotOptimize signature may be optimized away + // by the compiler. Cast const away to ensure we don't get that problem (and warning). + benchmark::DoNotOptimize(const_cast(active)); + benchmark::DoNotOptimize(const_cast(inactive)); benchmark::ClobberMemory(); } }