Simplify read templates
Clang build analyzer when run on bmcweb shows that
sdbusplus::message::read functions are one of the longer-to-compile
templates. Given that bmcweb has 400 ish method calls, and a variant
type[1] that's made grown adults cry in fear when presented with
compiler template errors, this should be something worth optimizing.
Luckily, we have c++17 these days, so we can vastly simplify our
unpacking code using variadic fold expressions[2]
As part of the simplification, this commit removes the whole grouping
class, as well as the class that creates std::tuples entirely, and
operates entirely on the variadic pack directly. This drops the
per-template compile time down to 700ms from 1800ms (note, these exact
times fluctuate a lot run to run and machine to machine).
For the grouping entries, there were only two users that I was able to
find. Ipmi[3] and ipmb. Running ipmitool in a fast loop shows no
measurable change in performance.
[1] https://github.com/openbmc/bmcweb/blob/cc67d0a0fed101c930b334a583d9ca9b222ceb77/include/dbus_utility.hpp#L32
[2] https://en.cppreference.com/w/cpp/language/fold
[3] https://github.com/openbmc/phosphor-host-ipmid/blob/920602f5f89b7aeeb7fd31dc77c768f1149cc1f2/ipmid-new.cpp#L493
Results from clang build analyzer after. Note, this is when isolated to
a single compile unit (redfish.cpp). When run across the full build and
all the template instantiations, this template saves 3-4 seconds on the
compile time.
```
735 ms: sdbusplus::message::message::read<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_string<cha... (1 times, avg 735 ms)
735 ms: sdbusplus::message::read<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_string<char>, std::... (1 times, avg 735 ms)
733 ms: sdbusplus::message::details::read_single<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_str... (1 times, avg 733 ms)
```
Before
```
1024 ms: sdbusplus::message::message::read<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_string<cha... (1 times, avg 1024 ms)
1024 ms: sdbusplus::message::read<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_string<char>, std::... (1 times, avg 1024 ms)
1022 ms: sdbusplus::message::details::read_grouping<std::tuple<>, std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tupl... (1 times, avg 1022 ms)
1017 ms: sdbusplus::message::details::read_tuple<std::tuple<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std:... (1 times, avg 1017 ms)
1015 ms: sdbusplus::message::details::read_single<std::vector<std::pair<std::basic_string<char>, std::variant<std::vector<std::tuple<std::basic_str... (1 times, avg 1015 ms)
```
Change-Id: I6445ec448f94db39c3bc4615e57a6cd3df421ca6
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/include/sdbusplus/message/read.hpp b/include/sdbusplus/message/read.hpp
index 84e9ee0..9d69bd5 100644
--- a/include/sdbusplus/message/read.hpp
+++ b/include/sdbusplus/message/read.hpp
@@ -42,67 +42,6 @@
namespace details
{
-/** @brief Utility to identify C++ types that may not be grouped into a
- * single sd_bus_message_read call and instead need special
- * handling.
- *
- * @tparam T - Type for identification.
- *
- * User-defined types are expected to inherit from std::false_type.
- * Enums are converted from strings, so must be done one at a time.
- *
- */
-template <typename T, typename Enable = void>
-struct can_read_multiple :
- std::conditional_t<std::is_enum_v<T>, std::false_type, std::true_type>
-{};
-// unix_fd's int needs to be wrapped
-template <>
-struct can_read_multiple<unix_fd> : std::false_type
-{};
-// std::string needs a char* conversion.
-template <>
-struct can_read_multiple<std::string> : std::false_type
-{};
-// object_path needs a char* conversion.
-template <>
-struct can_read_multiple<object_path> : std::false_type
-{};
-// signature needs a char* conversion.
-template <>
-struct can_read_multiple<signature> : std::false_type
-{};
-// bool needs to be resized to int, per sdbus documentation.
-template <>
-struct can_read_multiple<bool> : std::false_type
-{};
-
-// std::vector/map/unordered_vector/set need loops
-template <utility::has_emplace T>
-struct can_read_multiple<T> : std::false_type
-{};
-
-template <utility::has_emplace_back T>
-struct can_read_multiple<T> : std::false_type
-{};
-
-// std::pair needs to be broken down into components.
-template <typename T1, typename T2>
-struct can_read_multiple<std::pair<T1, T2>> : std::false_type
-{};
-
-// std::tuple needs to be broken down into components.
-template <typename... Args>
-struct can_read_multiple<std::tuple<Args...>> : std::false_type
-{};
-// variant needs to be broken down into components.
-template <typename... Args>
-struct can_read_multiple<std::variant<Args...>> : std::false_type
-{};
-
-template <typename... Args>
-inline constexpr bool can_read_multiple_v = can_read_multiple<Args...>::value;
-
/** @brief Utility to read a single C++ element from a sd_bus_message.
*
* User-defined types are expected to specialize this template in order to
@@ -456,162 +395,12 @@
static void op(sdbusplus::SdBusInterface*, sd_bus_message*, S&) {}
};
-template <typename T>
-void tuple_item_read(sdbusplus::SdBusInterface* intf, sd_bus_message* m, T&& t)
-{
- sdbusplus::message::read(intf, m, t);
-}
-
-template <int Index>
-struct ReadHelper
-{
- template <typename... Fields>
- static void op(sdbusplus::SdBusInterface* intf, sd_bus_message* m,
- std::tuple<Fields...> field_tuple)
- {
- auto& field = std::get<Index - 1>(field_tuple);
-
- if constexpr (Index > 1)
- {
- ReadHelper<Index - 1>::op(intf, m, std::move(field_tuple));
- }
-
- tuple_item_read(intf, m, field);
- }
-};
-
-/** @brief Read a tuple from the sd_bus_message.
- *
- * @tparam Tuple - The tuple type to read.
- * @param[out] t - The tuple to read into.
- *
- * A tuple of 2 or more entries can be read as a set with
- * sd_bus_message_read.
- *
- * A tuple of 1 entry can be read with sd_bus_message_read_basic.
- *
- * A tuple of 0 entries is a no-op.
- *
- */
-template <typename Tuple>
-void read_tuple(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t)
-{
- if constexpr (std::tuple_size_v<Tuple> >= 2)
- {
- ReadHelper<std::tuple_size_v<Tuple>>::op(intf, m, std::move(t));
- }
- else if constexpr (std::tuple_size_v<Tuple> == 1)
- {
- using itemType = decltype(std::get<0>(t));
- read_single_t<itemType>::op(intf, m,
- std::forward<itemType>(std::get<0>(t)));
- }
-}
-
-/** @brief Group a sequence of C++ types for reading from an sd_bus_message.
- * @tparam Tuple - A tuple of previously analyzed types.
- * @tparam Arg - The argument to analyze for grouping.
- *
- * Specialization for when can_read_multiple_v<Arg> is true.
- */
-template <typename Tuple, typename Arg>
-std::enable_if_t<can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg);
-/** @brief Group a sequence of C++ types for reading from an sd_bus_message.
- * @tparam Tuple - A tuple of previously analyzed types.
- * @tparam Arg - The argument to analyze for grouping.
- *
- * Specialization for when can_read_multiple_v<Arg> is false.
- */
-template <typename Tuple, typename Arg>
-std::enable_if_t<!can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg);
-/** @brief Group a sequence of C++ types for reading from an sd_bus_message.
- * @tparam Tuple - A tuple of previously analyzed types.
- * @tparam Arg - The argument to analyze for grouping.
- * @tparam Rest - The remaining arguments left to analyze.
- *
- * Specialization for when can_read_multiple_v<Arg> is true.
- */
-template <typename Tuple, typename Arg, typename... Rest>
-std::enable_if_t<can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg, Rest&&... rest);
-/** @brief Group a sequence of C++ types for reading from an sd_bus_message.
- * @tparam Tuple - A tuple of previously analyzed types.
- * @tparam Arg - The argument to analyze for grouping.
- * @tparam Rest - The remaining arguments left to analyze.
- *
- * Specialization for when can_read_multiple_v<Arg> is false.
- */
-template <typename Tuple, typename Arg, typename... Rest>
-std::enable_if_t<!can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg, Rest&&... rest);
-
-template <typename Tuple, typename Arg>
-std::enable_if_t<can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg)
-{
- // Last element of a sequence and can_read_multiple, so add it to
- // the tuple and call read_tuple.
-
- read_tuple(intf, m,
- std::tuple_cat(std::forward<Tuple>(t),
- std::forward_as_tuple(std::forward<Arg>(arg))));
-}
-
-template <typename Tuple, typename Arg>
-std::enable_if_t<!can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg)
-{
- // Last element of a sequence but !can_read_multiple, so call
- // read_tuple on the previous elements and separately this single
- // element.
-
- read_tuple(intf, m, std::forward<Tuple>(t));
- read_tuple(intf, m, std::forward_as_tuple(std::forward<Arg>(arg)));
-}
-
-template <typename Tuple, typename Arg, typename... Rest>
-std::enable_if_t<can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg, Rest&&... rest)
-{
- // Not the last element of a sequence and can_read_multiple, so add it
- // to the tuple and keep grouping.
-
- read_grouping(intf, m,
- std::tuple_cat(std::forward<Tuple>(t),
- std::forward_as_tuple(std::forward<Arg>(arg))),
- std::forward<Rest>(rest)...);
-}
-
-template <typename Tuple, typename Arg, typename... Rest>
-std::enable_if_t<!can_read_multiple_v<types::details::type_id_downcast_t<Arg>>>
- read_grouping(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Tuple&& t,
- Arg&& arg, Rest&&... rest)
-{
- // Not the last element of a sequence but !can_read_multiple, so call
- // read_tuple on the previous elements and separately this single
- // element and then group the remaining elements.
-
- read_tuple(intf, m, std::forward<Tuple>(t));
- read_tuple(intf, m, std::forward_as_tuple(std::forward<Arg>(arg)));
- read_grouping(intf, m, std::make_tuple(), std::forward<Rest>(rest)...);
-}
-
} // namespace details
template <typename... Args>
void read(sdbusplus::SdBusInterface* intf, sd_bus_message* m, Args&&... args)
{
- details::read_grouping(intf, m, std::make_tuple(),
- std::forward<Args>(args)...);
+ (details::read_single_t<Args>::op(intf, m, args), ...);
}
} // namespace message