optimization: use sd-bus array interface for array of integral types

Problem: Current sdbusplus will use sd_bus_message_read_basic and message_append_basic for decoding and encoding of arrays, no matter is the array element type is sized or unsized. This leads to some extreme performance downgrade in some cases, for example, passing a 4K bytes data array over sdbusplus interface now takes 4096 calls of sd_bus_message_read_basic and message_append_basic for each byte in the message, this will consume about 10ms CPU time on a Aspeed 2600 platform for each package on both send and receive side. Example callstack in MCTPD for decoding an incoming package with 4KB payload: ```sh (gdb) bt #0 sd_bus_message_read_basic (m=0x0, type=0 '\000', p=0x7ebd1438) at /usr/src/debug/systemd/1_253.3-r0/src/libsystemd/sd-bus/bus-message.c:2701 #1 0x004b4f38 in sdbusplus::message::details::read_single<unsigned char>::op<unsigned char&> (t=<optimized out>, m=0x54f234, intf=0x76f0ae44) at /usr/include/sdbusplus/message/read.hpp:144 #2 sdbusplus::message::details::read_tuple<std::tuple<unsigned char&> > (intf=intf@entry=0x76f17010 <sdbusplus::sdbus_impl>, m=m@entry=0x16faaa0, t=...) at /usr/include/sdbusplus/message/read.hpp:478 #3 0x004b5a20 in sdbusplus::message::details::read_grouping<std::tuple<>, unsigned char&>(sdbusplus::SdBusInterface*, sd_bus_message*, std::tuple<>&&, unsigned char&) ( arg=<optimized out>, t=..., m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:534 #4 sdbusplus::message::read<unsigned char&> (m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:585 #5 sdbusplus::message::details::tuple_item_read<unsigned char&> (t=<optimized out>, m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:434 #6 sdbusplus::message::details::ReadHelper<1>::op<unsigned char&, unsigned char&> (field_tuple=..., m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:451 #7 sdbusplus::message::details::ReadHelper<2>::op<unsigned char&, unsigned char&> (field_tuple=..., m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:448 #8 sdbusplus::message::details::read_tuple<std::tuple<unsigned char&, unsigned char&> > (t=..., m=0x16faaa0, intf=0x76f17010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/read.hpp:473 #9 sdbusplus::message::details::read_grouping<std::tuple<unsigned char&, unsigned char&>, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&> ( intf=0x76f17010 <sdbusplus::sdbus_impl>, m=0x16faaa0, t=..., arg=@0x7ebd1410: false) at /usr/include/sdbusplus/message/read.hpp:575 #10 0x0049cd80 in sdbusplus::message::details::read_grouping<std::tuple<unsigned char&>, unsigned char&, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&> ( arg=@0x7ebd1409: 0 '\000', t=..., m=<optimized out>, intf=<optimized out>) at /usr/include/sdbusplus/message/read.hpp:560 #11 sdbusplus::message::details::read_grouping<std::tuple<>, unsigned char&, unsigned char&, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&>(sdbusplus::SdBusInterface*, sd_bus_message*, std::tuple<>&&, unsigned char&, unsigned char&, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&) (arg=@0x7ebd140a: 0 '\000', t=..., m=<optimized out>, intf=<optimized out>) at /usr/include/sdbusplus/message/read.hpp:560 #12 sdbusplus::message::read<unsigned char&, unsigned char&, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&> (m=<optimized out>, intf=<optimized out>) at /usr/include/sdbusplus/message/read.hpp:585 #13 sdbusplus::message::message::read<unsigned char&, unsigned char&, bool&, std::vector<unsigned char, std::allocator<unsigned char> >&> (this=0x7ebd151c) at /usr/include/sdbusplus/message.hpp:156 #14 _ZZN9sdbusplus7utility15read_into_tupleISt5tupleIJhhbSt6vectorIhSaIhEEEEEEbRT_RNS_7message7messageEENKUlDpT_E_clIJSt17integral_constantIjLj0EESG_IjLj1EESG_IjLj2EESG_IjLj3EEEEEDaSD_ (__closure=<synthetic pointer>) at /usr/include/sdbusplus/utility/read_into_tuple.hpp:48 #15 _ZN9sdbusplus7utility6detail16index_apply_implIZNS0_15read_into_tupleISt5tupleIJhhbSt6vectorIhSaIhEEEEEEbRT_RNS_7message7messageEEUlDpT_E_JLj0ELj1ELj2ELj3EEEEDaS9_St16integer_sequenceIjJXspT0_EEE (f=...) at /usr/include/sdbusplus/utility/read_into_tuple.hpp:32 --Type <RET> for more, q to quit, c to continue without paging-- #16 _ZN9sdbusplus7utility6detail11index_applyILj4EZNS0_15read_into_tupleISt5tupleIJhhbSt6vectorIhSaIhEEEEEEbRT_RNS_7message7messageEEUlDpT_E_EEDaT0_ (f=...) at /usr/include/sdbusplus/utility/read_into_tuple.hpp:37 #17 sdbusplus::utility::read_into_tuple<std::tuple<unsigned char, unsigned char, bool, std::vector<unsigned char, std::allocator<unsigned char> > > > (m=..., t=...) at /usr/include/sdbusplus/utility/read_into_tuple.hpp:43 #18 sdbusplus::asio::callback_method_instance<MctpBinding::MctpBinding(std::shared_ptr<sdbusplus::asio::connection>, std::shared_ptr<sdbusplus::asio::object_server>&, const std::string&, const Configuration&, boost::asio::io_context&, sdbusplus::common::xyz::openbmc_project::mctp::Base::BindingTypes)::<lambda(uint8_t, uint8_t, bool, std::vector<unsigned char, std::allocator<unsigned char> >)> >::operator() (m=..., this=0x16ec278) at /usr/include/sdbusplus/asio/object_server.hpp:193 #19 std::__invoke_impl<int, sdbusplus::asio::callback_method_instance<MctpBinding::MctpBinding(std::shared_ptr<sdbusplus::asio::connection>, std::shared_ptr<sdbusplus::asio::object_server>&, const std::string&, const Configuration&, boost::asio::io_context&, sdbusplus::common::xyz::openbmc_project::mctp::Base::BindingTypes)::<lambda(uint8_t, uint8_t, bool, std::vector<unsigned char>)> >&, sdbusplus::message::message&> (__f=...) at /usr/include/c++/13.1.1/bits/invoke.h:61 #20 std::__invoke_r<int, sdbusplus::asio::callback_method_instance<MctpBinding::MctpBinding(std::shared_ptr<sdbusplus::asio::connection>, std::shared_ptr<sdbusplus::asio::object_server>&, const std::string&, const Configuration&, boost::asio::io_context&, sdbusplus::common::xyz::openbmc_project::mctp::Base::BindingTypes)::<lambda(uint8_t, uint8_t, bool, std::vector<unsigned char>)> >&, sdbusplus::message::message&> (__fn=...) at /usr/include/c++/13.1.1/bits/invoke.h:114 #21 std::_Function_handler<int(sdbusplus::message::message&), sdbusplus::asio::callback_method_instance<MctpBinding::MctpBinding(std::shared_ptr<sdbusplus::asio::connection>, std::shared_ptr<sdbusplus::asio::object_server>&, const std::string&, const Configuration&, boost::asio::io_context&, sdbusplus::common::xyz::openbmc_project::mctp::Base::BindingTypes)::<lambda(uint8_t, uint8_t, bool, std::vector<unsigned char, std::allocator<unsigned char> >)> > >::_M_invoke(const std::_Any_data &, sdbusplus::message::message &) (__functor=..., __args#0=...) at /usr/include/c++/13.1.1/bits/std_function.h:290 #22 0x004a192c in std::function<int (sdbusplus::message::message&)>::operator()(sdbusplus::message::message&) const (__args#0=..., this=0x16ec278) at /usr/include/c++/13.1.1/bits/std_function.h:591 #23 sdbusplus::asio::dbus_interface::method_handler (m=<optimized out>, userdata=0x16ec260, error=0x7ebd1578) at /usr/include/sdbusplus/asio/object_server.hpp:700 #24 0x76d7ca68 in method_callbacks_run (bus=0x16ea950, m=0x16faaa0, c=0x16ed010, require_fallback=false, found_object=0x7ebd160b) at /usr/src/debug/systemd/1_253.3-r0/src/libsystemd/sd-bus/bus-objects.c:406 #25 0x76d805d8 in object_find_and_run (bus=0x16ea950, m=0x16faaa0, p=0x16f99e0 "/xyz/openbmc_project/mctp", require_fallback=false, found_object=0x7ebd160b) at /usr/src/debug/systemd/1_253.3-r0/src/libsystemd/sd-bus/bus-objects.c:1319 #26 0x76d80e18 in bus_process_object (bus=0x16ea950, m=0x16faaa0) at /usr/src/debug/systemd/1_253.3-r0/src/libsystemd/sd-bus/bus-objects.c:1439 #27 0x76da3b9c in process_message (bus=0x16ea950, m=0x16faaa0) at /usr/src/debug/systemd/1_253.3-r0/src/libsystemd/sd-bus/sd-bus.c:2976 ``` Similarly for PLDMD to send a package with 4KB data payload: ```sh (gdb) bt #0 message_append_basic (m=0x7a84580, type=121 'y', p=0x7a8ad03, stored=0x0) at /usr/src/debug/sdbus-c++-libsystemd/250.9-r0/src/libsystemd/sd-bus/bus-message.c:1583 #1 0x76f1603c in sdbusplus::message::details::append_single<unsigned char, void>::op<unsigned char const&> (t=<optimized out>, m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:165 #2 sdbusplus::message::details::append_tuple<std::tuple<unsigned char const&> > (intf=intf@entry=0x76ede010 <sdbusplus::sdbus_impl>, m=m@entry=0x7a84580, t=...) at /usr/include/sdbusplus/message/append.hpp:415 #3 0x76f160c0 in sdbusplus::message::details::append_grouping<std::tuple<>, unsigned char const&>(sdbusplus::SdBusInterface*, sd_bus_message*, std::tuple<>&&, unsigned char const&) (arg=@0x7a8ad03: 46 '.', t=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:481 #4 sdbusplus::message::append<unsigned char const&> (m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:535 #5 sdbusplus::message::details::append_single<std::vector<unsigned char, std::allocator<unsigned char> >, void>::op<std::vector<unsigned char, std::allocator<unsigned char> > const&> (s=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:281 #6 sdbusplus::message::details::append_tuple<std::tuple<std::vector<unsigned char, std::allocator<unsigned char> > const&> > (intf=intf@entry=0x76ede010 <sdbusplus::sdbus_impl>, m=m@entry=0x7a84580, t=...) at /usr/include/sdbusplus/message/append.hpp:415 #7 0x76f161bc in sdbusplus::message::details::append_grouping<std::tuple<>, std::vector<unsigned char, std::allocator<unsigned char> > const&>(sdbusplus::SdBusInterface*, sd_bus_message*, std::tuple<>&&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (arg=..., t=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:497 #8 sdbusplus::message::details::append_grouping<std::tuple<unsigned char const&, unsigned char const&>, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&> (arg=@0x7aa3eb0: 240, t=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:527 #9 sdbusplus::message::details::append_grouping<std::tuple<unsigned char const&>, unsigned char const&, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&> (arg=@0x7a846ec: 0 '\000', t=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:508 #10 sdbusplus::message::details::append_grouping<std::tuple<>, unsigned char const&, unsigned char const&, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&>(sdbusplus::SdBusInterface*, sd_bus_message*, std::tuple<>&&, unsigned char const&, unsigned char const&, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&) (arg=@0x76f4a10d: 164 '\244', t=..., m=0x7a84580, intf=0x76ede010 <sdbusplus::sdbus_impl>) at /usr/include/sdbusplus/message/append.hpp:508 #11 sdbusplus::message::append<unsigned char const&, unsigned char const&, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&> ( intf=0x76ede010 <sdbusplus::sdbus_impl>, m=0x7a84580) at /usr/include/sdbusplus/message/append.hpp:535 #12 0x76f2bd68 in sdbusplus::message::message::append<unsigned char const&, unsigned char const&, bool const&, std::vector<unsigned char, std::allocator<unsigned char> > const&> ( this=0x7aa3db4) at /usr/include/c++/13.1.1/bits/unique_ptr.h:199 #13 sdbusplus::asio::connection::yield_method_call<int, unsigned char, unsigned char, bool, std::vector<unsigned char, std::allocator<unsigned char> > > ( this=this@entry=0x7a7719c, yield=..., ec=..., service=..., objpath=..., interf=..., method=...) at /usr/include/sdbusplus/asio/connection.hpp:240 #14 0x76f042bc in mctpw::MCTPImpl::sendYield (this=<optimized out>, yield=..., dstEId=<optimized out>, dstEId@entry=9 '\t', msgTag=<optimized out>, msgTag@entry=84 'T', tagOwner=<optimized out>, tagOwner@entry=172, request=...) at /usr/src/debug/mctpwplus/1.0+git999-r0/mctpwplus/mctp_impl.cpp:612 #15 0x76ef3e28 in mctpw::MCTPWrapper::sendYield (this=<optimized out>, yield=..., dstEId=dstEId@entry=9 '\t', msgTag=msgTag@entry=0 '\000', tagOwner=tagOwner@entry=false, --Type <RET> for more, q to quit, c to continue without paging-- request=...) at /usr/src/debug/mctpwplus/1.0+git999-r0/mctpwplus/mctp_wrapper.cpp:128 #16 0x004dccac in pldm::sendPldmMessageEid (yield=..., eid=eid@entry=9 '\t', retryCount=retryCount@entry=3 '\003', msgTag=msgTag@entry=0 '\000', tagOwner=6, tagOwner@entry=false, payload=<error reading variable: Cannot access memory at address 0x0>, type=<optimized out>) at /usr/include/c++/13.1.1/bits/unique_ptr.h:19 ``` Solution: When the vector or array's element type is trivially copyable, and is an integral type, then use sd_bus_message_append_array_space and sd_bus_message_read_array to do the message encode/decode instead. Test: Tested with intensive D-Bus transactions, which has large byte array as IPC message payload. The encode and decode time is negligible now compare to the IPC delay (down from 10ms to less than 1ms) Change-Id: I6235ce27fc50dd7317ecf0a48fa3340c9fbc8d3b Signed-off-by: Yongbing Chen <[email protected]>
openbmc · Jan 6, 2024 · 3bc5359 · 3bc5359
1 parent f36d903
commit 3bc5359
Show file tree

Hide file tree

Showing 6 changed files with 222 additions and 25 deletions.
diff --git a/include/sdbusplus/message/append.hpp b/include/sdbusplus/message/append.hpp
@@ -264,9 +264,36 @@ struct append_single<bool>
     }
 };
 
+/** @brief Helper templates to tell if T is a vector or array
+ */
+template <typename T>
+struct is_vector : std::false_type
+{};
+template <typename... Args>
+struct is_vector<std::vector<Args...>> : std::true_type
+{};
+template <typename T>
+struct is_array : std::false_type
+{};
+template <typename T, std::size_t N>
+struct is_array<std::array<T, N>> : std::true_type
+{};
+template <typename T>
+inline constexpr bool is_vector_v = is_vector<T>::value;
+template <typename T>
+inline constexpr bool is_array_v = is_array<T>::value;
+template <typename T>
+struct is_vector_or_array : std::bool_constant<is_vector_v<T> || is_array_v<T>>
+{};
+
 /** @brief Specialization of append_single for containers (ie vector, array,
- * set, map, etc) */
+ * set, map, etc), for T is not vector or array, or its elements is not
+ * trivially copyable, or is not an integral type*/
 template <typename T>
+    requires(
+        !std::conjunction<is_vector_or_array<T>,
+                          std::is_trivially_copyable<typename T::value_type>,
+                          std::is_integral<typename T::value_type>>::value)
 struct append_single<T, std::enable_if_t<utility::has_const_iterator_v<T>>>
 {
     template <typename S>
@@ -284,6 +311,31 @@ struct append_single<T, std::enable_if_t<utility::has_const_iterator_v<T>>>
     }
 };
 
+/** @brief Specialization of append_single for vector and array T,
+ * with its elements is trivially copyable, and is an integral type*/
+template <typename T>
+    requires(
+        std::conjunction<is_vector_or_array<T>,
+                         std::is_trivially_copyable<typename T::value_type>,
+                         std::is_integral<typename T::value_type>>::value)
+struct append_single<T, std::enable_if_t<utility::has_const_iterator_v<T>>>
+{
+    template <typename S>
+    static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
+    {
+        constexpr auto dbusType = utility::tuple_to_array(types::type_id<T>());
+
+        uint8_t* p;
+        size_t elementSize = bus_type_get_size(dbusType[1]);
+        size_t sz = s.size() * elementSize;
+        intf->sd_bus_message_append_array_space(m, dbusType[1], sz, (void**)&p);
+        for (size_t i = 0; i < s.size(); i++)
+        {
+            memcpy(p + i * elementSize, &s[i], elementSize);
+        }
+    }
+};
+
 /** @brief Specialization of append_single for std::pairs. */
 template <typename T1, typename T2>
 struct append_single<std::pair<T1, T2>>

diff --git a/include/sdbusplus/message/read.hpp b/include/sdbusplus/message/read.hpp
@@ -212,9 +212,14 @@ struct read_single<S>
     }
 };
 
-/** @brief Specialization of read_single for std::vectors. */
+/** @brief Specialization of read_single for std::vectors, with its elements is
+ * not trivially copyable, or is not an integral type.
+ */
 template <typename S>
-    requires(utility::has_emplace_back_method_v<S>)
+    requires(
+        utility::has_emplace_back_method_v<S> &&
+        !std::conjunction<std::is_trivially_copyable<typename S::value_type>,
+                          std::is_integral<typename S::value_type>>::value)
 struct read_single<S>
 {
     template <typename T>
@@ -250,6 +255,40 @@ struct read_single<S>
     }
 };
 
+/** @brief Specialization of read_single for std::vectors, with its elements is
+ * trivially copyable, and is an integral type
+ */
+template <typename S>
+    requires(
+        std::conjunction_v<utility::has_emplace_back_method<S>,
+                           std::is_trivially_copyable<typename S::value_type>,
+                           std::is_integral<typename S::value_type>>)
+struct read_single<S>
+{
+    template <typename T>
+    static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m, T&& t)
+    {
+        size_t sz;
+        const void* p;
+        constexpr auto dbusType = utility::tuple_to_array(types::type_id<S>());
+        int r = intf->sd_bus_message_read_array(m, dbusType[1], &p, &sz);
+        if (r < 0)
+        {
+            throw exception::SdBusError(-r, "sd_bus_message_read_array");
+        }
+
+        size_t elementSize = bus_type_get_size(dbusType[1]);
+        size_t numElements = sz / elementSize;
+        for (size_t i = 0; i < numElements; ++i)
+        {
+            types::details::type_id_downcast_t<typename S::value_type> s;
+            memcpy(&s, static_cast<const char*>(p) + i * elementSize,
+                   elementSize);
+            t.emplace_back(s);
+        }
+    }
+};
+
 /** @brief Specialization of read_single for std::map. */
 template <typename S>
     requires(utility::has_emplace_method_v<S>)

diff --git a/include/sdbusplus/sdbus.hpp b/include/sdbusplus/sdbus.hpp
@@ -12,6 +12,33 @@ namespace sdbusplus
 using SdBusDuration =
     std::chrono::duration<uint64_t, std::chrono::microseconds::period>;
 
+// This is a copy from sd-bus.h
+static inline int bus_type_get_size(char c)
+{
+    switch (c)
+    {
+        case SD_BUS_TYPE_BYTE:
+            return 1;
+
+        case SD_BUS_TYPE_INT16:
+        case SD_BUS_TYPE_UINT16:
+            return 2;
+
+        case SD_BUS_TYPE_BOOLEAN:
+        case SD_BUS_TYPE_INT32:
+        case SD_BUS_TYPE_UINT32:
+        case SD_BUS_TYPE_UNIX_FD:
+            return 4;
+
+        case SD_BUS_TYPE_INT64:
+        case SD_BUS_TYPE_UINT64:
+        case SD_BUS_TYPE_DOUBLE:
+            return 8;
+    }
+
+    return -EINVAL;
+}
+
 // A wrapper for interfacing or testing sd-bus.  This will hold methods for
 // buses, messages, etc.
 class SdBusInterface
@@ -177,6 +204,11 @@ class SdBusInterface
     virtual int sd_bus_is_open(sd_bus* bus) = 0;
 
     virtual int sd_bus_wait(sd_bus* bus, uint64_t timeout_usec) = 0;
+
+    virtual int sd_bus_message_append_array_space(sd_bus_message* m, char type,
+                                                  size_t size, void** ptr) = 0;
+    virtual int sd_bus_message_read_array(sd_bus_message* m, char type,
+                                          const void** ptr, size_t* size) = 0;
 };
 
 class SdBusImpl : public SdBusInterface
@@ -571,6 +603,18 @@ class SdBusImpl : public SdBusInterface
     {
         return ::sd_bus_wait(bus, timeout_usec);
     }
+
+    int sd_bus_message_append_array_space(sd_bus_message* m, char type,
+                                          size_t size, void** ptr) override
+    {
+        return ::sd_bus_message_append_array_space(m, type, size, ptr);
+    }
+
+    int sd_bus_message_read_array(sd_bus_message* m, char type,
+                                  const void** ptr, size_t* size) override
+    {
+        return ::sd_bus_message_read_array(m, type, ptr, size);
+    }
 };
 
 extern SdBusImpl sdbus_impl;

diff --git a/include/sdbusplus/test/sdbus_mock.hpp b/include/sdbusplus/test/sdbus_mock.hpp
@@ -162,6 +162,10 @@ class SdBusMock : public SdBusInterface
     MOCK_METHOD(void, sd_bus_close, (sd_bus*), (override));
     MOCK_METHOD(int, sd_bus_is_open, (sd_bus*), (override));
     MOCK_METHOD(int, sd_bus_wait, (sd_bus*, uint64_t), (override));
+    MOCK_METHOD(int, sd_bus_message_append_array_space,
+                (sd_bus_message*, char, size_t, void**), (override));
+    MOCK_METHOD(int, sd_bus_message_read_array,
+                (sd_bus_message*, char, const void**, size_t*), (override));
 
     SdBusMock()
     {

diff --git a/test/message/append.cpp b/test/message/append.cpp
@@ -90,6 +90,17 @@ class AppendTest : public testing::Test
         EXPECT_CALL(mock, sd_bus_message_close_container(nullptr))
             .WillOnce(Return(0));
     }
+
+    void expect_append_array_space(char type, size_t sz)
+    {
+        EXPECT_CALL(mock, sd_bus_message_append_array_space(nullptr, type, sz,
+                                                            testing::_))
+            .WillOnce(testing::Invoke(
+                [sz](sd_bus_message*, char, size_t, void** p) -> int {
+            *p = malloc(sz);
+            return 0;
+        }));
+    }
 };
 
 TEST_F(AppendTest, RValueInt)
@@ -285,20 +296,27 @@ TEST_F(AppendTest, Span)
 
 TEST_F(AppendTest, Vector)
 {
-    const std::vector<int> v{1, 2, 3, 4};
+    const std::vector<std::string> v{"1", "2", "3", "4"};
 
     {
         testing::InSequence seq;
-        expect_open_container(SD_BUS_TYPE_ARRAY, "i");
+        expect_open_container(SD_BUS_TYPE_ARRAY, "s");
         for (const auto& i : v)
         {
-            expect_basic<int>(SD_BUS_TYPE_INT32, i);
+            expect_basic_string(SD_BUS_TYPE_STRING, i.c_str());
         }
         expect_close_container();
     }
     new_message().append(v);
 }
 
+TEST_F(AppendTest, VectorIntegral)
+{
+    const std::vector<int> v{1, 2, 3, 4};
+    expect_append_array_space(SD_BUS_TYPE_INT32, v.size() * sizeof(int));
+    new_message().append(v);
+}
+
 TEST_F(AppendTest, Set)
 {
     const std::set<std::string> s{"one", "two", "eight"};