Skip to content

Commit c0d72c3

Browse files
authored
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 <[email protected]>
1 parent 6e97990 commit c0d72c3

File tree

2 files changed

+20
-13
lines changed

2 files changed

+20
-13
lines changed

rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_client.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ class BenchmarkLifecycleClient : public performance_test_fixture::PerformanceTes
228228
BENCHMARK_F(BenchmarkLifecycleClient, get_state)(benchmark::State & state) {
229229
for (auto _ : state) {
230230
(void)_;
231-
const auto lifecycle_state = lifecycle_client->get_state();
231+
232+
lifecycle_msgs::msg::State lifecycle_state = lifecycle_client->get_state();
232233
if (lifecycle_state.id != lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED) {
233234
const std::string msg =
234235
std::string("Expected state did not match actual: ") +
@@ -268,7 +269,7 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_available_states)(benchmark::State & s
268269
for (auto _ : state) {
269270
(void)_;
270271
constexpr size_t expected_states = 11u;
271-
const auto states = lifecycle_client->get_available_states();
272+
std::vector<lifecycle_msgs::msg::State> states = lifecycle_client->get_available_states();
272273
if (states.size() != expected_states) {
273274
const std::string msg =
274275
std::string("Expected number of states did not match actual: ") +
@@ -284,7 +285,8 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_available_transitions)(benchmark::Stat
284285
for (auto _ : state) {
285286
(void)_;
286287
constexpr size_t expected_transitions = 2u;
287-
const auto transitions = lifecycle_client->get_available_transitions();
288+
std::vector<lifecycle_msgs::msg::TransitionDescription> transitions =
289+
lifecycle_client->get_available_transitions();
288290
if (transitions.size() != expected_transitions) {
289291
const std::string msg =
290292
std::string("Expected number of transitions did not match actual: ") +
@@ -300,7 +302,8 @@ BENCHMARK_F(BenchmarkLifecycleClient, get_transition_graph)(benchmark::State & s
300302
for (auto _ : state) {
301303
(void)_;
302304
constexpr size_t expected_transitions = 25u;
303-
const auto transitions = lifecycle_client->get_transition_graph();
305+
std::vector<lifecycle_msgs::msg::TransitionDescription> transitions =
306+
lifecycle_client->get_transition_graph();
304307
if (transitions.size() != expected_transitions) {
305308
const std::string msg =
306309
std::string("Expected number of transitions did not match actual: ") +

rclcpp_lifecycle/test/benchmark/benchmark_lifecycle_node.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,15 @@ class BenchmarkLifecycleNode : public performance_test_fixture::PerformanceTest
9797
BENCHMARK_F(BenchmarkLifecycleNode, get_current_state)(benchmark::State & state) {
9898
for (auto _ : state) {
9999
(void)_;
100-
const auto & lifecycle_state = node->get_current_state();
100+
const rclcpp_lifecycle::State & lifecycle_state = node->get_current_state();
101101
if (lifecycle_state.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED) {
102102
const std::string message =
103103
std::string("Node's current state is: ") + std::to_string(lifecycle_state.id());
104104
state.SkipWithError(message.c_str());
105105
}
106-
benchmark::DoNotOptimize(lifecycle_state);
106+
// Google benchmark 1.8.2 warns that the constref DoNotOptimize signature may be optimized away
107+
// by the compiler. Cast const away to ensure we don't get that problem (and warning).
108+
benchmark::DoNotOptimize(const_cast<rclcpp_lifecycle::State &>(lifecycle_state));
107109
benchmark::ClobberMemory();
108110
}
109111
}
@@ -112,7 +114,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_available_states)(benchmark::State & sta
112114
for (auto _ : state) {
113115
(void)_;
114116
constexpr size_t expected_states = 11u;
115-
const auto lifecycle_states = node->get_available_states();
117+
std::vector<rclcpp_lifecycle::State> lifecycle_states = node->get_available_states();
116118
if (lifecycle_states.size() != expected_states) {
117119
const std::string msg = std::to_string(lifecycle_states.size());
118120
state.SkipWithError(msg.c_str());
@@ -126,7 +128,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_available_transitions)(benchmark::State
126128
for (auto _ : state) {
127129
(void)_;
128130
constexpr size_t expected_transitions = 2u;
129-
const auto & transitions = node->get_available_transitions();
131+
std::vector<rclcpp_lifecycle::Transition> transitions = node->get_available_transitions();
130132
if (transitions.size() != expected_transitions) {
131133
const std::string msg = std::to_string(transitions.size());
132134
state.SkipWithError(msg.c_str());
@@ -140,7 +142,7 @@ BENCHMARK_F(BenchmarkLifecycleNode, get_transition_graph)(benchmark::State & sta
140142
for (auto _ : state) {
141143
(void)_;
142144
constexpr size_t expected_transitions = 25u;
143-
const auto & transitions = node->get_transition_graph();
145+
std::vector<rclcpp_lifecycle::Transition> transitions = node->get_transition_graph();
144146
if (transitions.size() != expected_transitions) {
145147
const std::string msg =
146148
std::string("Expected number of transitions did not match actual: ") +
@@ -162,18 +164,20 @@ BENCHMARK_F(BenchmarkLifecycleNode, transition_valid_state)(benchmark::State & s
162164
reset_heap_counters();
163165
for (auto _ : state) {
164166
(void)_;
165-
const auto & active =
167+
const rclcpp_lifecycle::State & active =
166168
node->trigger_transition(lifecycle_msgs::msg::Transition::TRANSITION_ACTIVATE);
167169
if (active.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE) {
168170
state.SkipWithError("Transition to active state failed");
169171
}
170-
const auto & inactive =
172+
const rclcpp_lifecycle::State & inactive =
171173
node->trigger_transition(lifecycle_msgs::msg::Transition::TRANSITION_DEACTIVATE);
172174
if (inactive.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE) {
173175
state.SkipWithError("Transition to inactive state failed");
174176
}
175-
benchmark::DoNotOptimize(active);
176-
benchmark::DoNotOptimize(inactive);
177+
// Google benchmark 1.8.2 warns that the constref DoNotOptimize signature may be optimized away
178+
// by the compiler. Cast const away to ensure we don't get that problem (and warning).
179+
benchmark::DoNotOptimize(const_cast<rclcpp_lifecycle::State &>(active));
180+
benchmark::DoNotOptimize(const_cast<rclcpp_lifecycle::State &>(inactive));
177181
benchmark::ClobberMemory();
178182
}
179183
}

0 commit comments

Comments
 (0)