Use sd-bus array methods for read and append
Currently sdbusplus uses sd_bus_message_read_basic and
message_append_basic for decoding and encoding of arrays. For large
arrays, this imposes a significant overhead in calling the library each
time. This leads to some extreme performance degradation in some cases,
for example, passing a 4K bytes data array over sdbusplus interface now
takes 4096 + 2 calls of sd_bus_message_read_basic and
message_append_basic, this will consume about 10ms CPU time on a Aspeed
2600 platform for each package on both send and receive side.
While in this case, a DBus interface design should likely opt for using
an FD rather than an array of bytes, this isn't a reason to not optimize
this case.
sd-bus, in version 240 added methods to deal with this performance
degradation, namely sd_bus_message_read_array and
sd_bus_message_append_array, which each only require a single call to
load values into the array using pointers and lengths.
This patchset is based on the one submitted here:
https://gerrit.openbmc.org/c/openbmc/sdbusplus/+/68614
But opts for a different approach to a number of technical details on
how it accomplishes the result, including changing the underlying sd-bus
calls used to utilize ones that don't require sdbus internal malloc,
avoiding unit tests ASAN issues in the process.
This commit adds support for utilizing the above methods when doing
calls to the library if the following requirements are met:
1. The container is contiguous (using the std::contiguous_iterator
concept)
2. The container represents a list of values of type int/uint 8,16,32,
or 64 bits wide. Note, per the sd-bus documentation, arrays of bool are
explicitly not supported (presumably because the internal representation
of a bool array for both libc++ and sd-bus is implementation defined).
To accomplish this, the arrays handling for contiguous arrays is moved
to use concepts, simplifying the code significantly, and allowing the
use of std::contiguous_iterator.
Change-Id: I3bd9f97ed160835e8c30c302b80553a1d450bbf9
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Yongbing Chen <yongbingchen@google.com>
diff --git a/test/message/append.cpp b/test/message/append.cpp
index e753b01..8e0e558 100644
--- a/test/message/append.cpp
+++ b/test/message/append.cpp
@@ -90,6 +90,18 @@
EXPECT_CALL(mock, sd_bus_message_close_container(nullptr))
.WillOnce(Return(0));
}
+
+ static int on_array_append(sd_bus_message*, char, const void*, size_t)
+ {
+ return 0;
+ }
+
+ void expect_append_array(char type, size_t sz)
+ {
+ EXPECT_CALL(mock,
+ sd_bus_message_append_array(nullptr, type, testing::_, sz))
+ .WillOnce(testing::Invoke(on_array_append));
+ }
};
TEST_F(AppendTest, RValueInt)
@@ -253,13 +265,7 @@
!sdbusplus::message::details::can_append_multiple_v<decltype(a)>);
{
- testing::InSequence seq;
- expect_open_container(SD_BUS_TYPE_ARRAY, "d");
- for (const auto& i : a)
- {
- expect_basic<double>(SD_BUS_TYPE_DOUBLE, i);
- }
- expect_close_container();
+ expect_append_array(SD_BUS_TYPE_DOUBLE, a.size() * sizeof(double));
}
new_message().append(a);
}
@@ -272,13 +278,7 @@
!sdbusplus::message::details::can_append_multiple_v<decltype(s)>);
{
- testing::InSequence seq;
- expect_open_container(SD_BUS_TYPE_ARRAY, "d");
- for (const auto& i : s)
- {
- expect_basic<double>(SD_BUS_TYPE_DOUBLE, i);
- }
- expect_close_container();
+ expect_append_array(SD_BUS_TYPE_DOUBLE, a.size() * sizeof(double));
}
new_message().append(s);
}
@@ -299,6 +299,31 @@
new_message().append(v);
}
+TEST_F(AppendTest, VectorIntegral)
+{
+ const std::vector<int32_t> v{1, 2, 3, 4};
+ expect_append_array(SD_BUS_TYPE_INT32, v.size() * sizeof(int32_t));
+ new_message().append(v);
+}
+
+TEST_F(AppendTest, VectorNestIntegral)
+{
+ const std::vector<std::array<int32_t, 3>> v{
+ {1, 2, 3}, {3, 4, 5}, {6, 7, 8}};
+
+ {
+ testing::InSequence seq;
+ expect_open_container(SD_BUS_TYPE_ARRAY, "ai");
+ for (long unsigned int i = 0; i < v.size(); i++)
+ {
+ expect_append_array(SD_BUS_TYPE_INT32,
+ v[i].size() * sizeof(int32_t));
+ }
+ expect_close_container();
+ }
+ new_message().append(v);
+}
+
TEST_F(AppendTest, Set)
{
const std::set<std::string> s{"one", "two", "eight"};