-
Notifications
You must be signed in to change notification settings - Fork 125
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 to rosidl_runtime_cpp bounded vector #528
Conversation
Signed-off-by: ahcorde <ahcorde@gmail.com>
@cottsay Does it make sense to add a new folder for the benchmark tests? |
Benchmark job: (job deleted)
I think that pre-allocating a static 4096 size in the vector is really making the |
the BoundedVector class is templated: rosidl_runtime_cpp::BoundedVector<int, 4096> v; This required a |
Okay, I have a better understanding of what's going on now. My interpretation of the template arguments was wrong, but we should still be pre-allocating, or it will skew the results on the first iteration. Looks like On a separate note, you should drop the calls to |
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're still not reserving capacity in the vector before starting the timing loop, so there is still a single allocation happening for the first iteration but not for any subsequent iterations, resulting in a really small value for the heap_allocations
value in the results.
All but one of these tests act in this way. The bounded_vector_input_iterators
test, however, is performing an allocation for every assign
invocation, which is interesting (this is demonstrated by heap_allocations=1
in the results, meaning that on average, there was 1 allocation for every iteration of the timing loop). My guess is that there is an unconditional call to realloc
that's probably not in our code.
This is the patch I used to resolve that:
--- a/rosidl_runtime_cpp/test/benchmark/benchmark_bounded_vector.cpp
+++ b/rosidl_runtime_cpp/test/benchmark/benchmark_bounded_vector.cpp
@@ -28,6 +28,10 @@ BENCHMARK_DEFINE_F(PerformanceTest, bounded_vector)(benchmark::State & st)
{
rosidl_runtime_cpp::BoundedVector<int, 1> v;
+ v.reserve(1);
+
+ reset_heap_counters();
+
for (auto _ : st) {
v.push_back(0);
v.erase(v.begin());
@@ -39,6 +43,10 @@ BENCHMARK_DEFINE_F(PerformanceTest, bounded_vector_rvalue)(benchmark::State & st
{
rosidl_runtime_cpp::BoundedVector<int, 1> v;
+ v.reserve(1);
+
+ reset_heap_counters();
+
for (auto _ : st) {
v.emplace_back(0);
v.erase(v.begin());
@@ -49,9 +57,10 @@ BENCHMARK_REGISTER_F(PerformanceTest, bounded_vector_rvalue);
BENCHMARK_DEFINE_F(PerformanceTest, bounded_vector_insert)(benchmark::State & st)
{
rosidl_runtime_cpp::BoundedVector<int, 1> v;
- rosidl_runtime_cpp::BoundedVector<int, 1> v2;
+ rosidl_runtime_cpp::BoundedVector<int, 1> v2;
v2.push_back(0);
+ v.reserve(1);
reset_heap_counters();
@@ -71,6 +80,7 @@ BENCHMARK_DEFINE_F(PerformanceTest, bounded_vector_input_iterators)(benchmark::S
std::istringstream ss;
ss.str(vector_string);
std::istream_iterator<int> ii(ss), end;
+ v.reserve(1);
reset_heap_counters();
@@ -87,6 +97,9 @@ BENCHMARK_DEFINE_F(PerformanceTest, bounded_vector_forward_iterators)(benchmark:
std::forward_list<int> l;
l.push_front(0);
+ v.reserve(1);
+
+ reset_heap_counters();
for (auto _ : st) {
v.assign(l.begin(), l.end());
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Alejandro Hernández Cordero <alejandro@openrobotics.org>
This PR builds on top of this other PR #521. Added some benchmark test to the
BoundedVector
classCompiling up-to
rosidl_runtime_cpp
and testingrosidl_runtime_cpp
with-DAMENT_RUN_PERFORMANCE_TESTS=ON
:Signed-off-by: ahcorde ahcorde@gmail.com