Optimize object_server
There are quite a few optimizations that can be made to object_server to
reduce memory usage, and improve simplicity. First, the virtualized
classes have been removed in exchange for std::function based type
erasure.
All the underlying structures have been moved to std::vector, to improve
memory packing and locality. To accomplish this, the vtable
construction is moved into the initialize function, such that when
initialize is called, the dbus_interface object knows there will be no
more changes, and can therefore take pointers to elements within the
vector for the vtable.
The independent storage of string names has been removed in lieu of
dedicated structures, signal, method_callback, and property_callback,
which each represent one of the primitive types, and each of which have
a "name" string that can be used, whereas previously name required
duplication across each of the unordered_maps.
Change-Id: I0236f1b85c53f1b5d05931ea100c04de6e571665
Signed-off-by: Ed Tanous <edtanous@google.com>
diff --git a/include/sdbusplus/asio/object_server.hpp b/include/sdbusplus/asio/object_server.hpp
index 46c4191..3484d9c 100644
--- a/include/sdbusplus/asio/object_server.hpp
+++ b/include/sdbusplus/asio/object_server.hpp
@@ -5,7 +5,6 @@
// but by defining it here, warnings won't cause problems with a compile
#define BOOST_COROUTINES_NO_DEPRECATION_WARNING
#endif
-
#include <boost/asio/spawn.hpp>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/exception.hpp>
@@ -16,23 +15,16 @@
#include <sdbusplus/utility/type_traits.hpp>
#include <any>
-#include <list>
+#include <functional>
#include <optional>
-#include <set>
-#include <unordered_map>
+#include <utility>
+#include <vector>
namespace sdbusplus
{
namespace asio
{
-class callback
-{
- public:
- virtual ~callback() = default;
- virtual int call(message_t& m) = 0;
-};
-
enum class SetPropertyReturnValue
{
fail = 0,
@@ -40,12 +32,31 @@
sameValueUpdated,
};
-class callback_set
+class property_callback
{
public:
- virtual ~callback_set() = default;
- virtual SetPropertyReturnValue call(message_t& m) = 0;
- virtual SetPropertyReturnValue set(const std::any& value) = 0;
+ std::string name;
+ std::function<int(message_t&)> on_get;
+ std::function<SetPropertyReturnValue(message_t&)> on_set_message;
+ std::function<SetPropertyReturnValue(const std::any&)> on_set_value;
+ const char* signature;
+ decltype(vtable_t::flags) flags;
+};
+
+class method_callback
+{
+ public:
+ std::string name;
+ std::function<int(message_t&)> call;
+ const char* arg_signature;
+ const char* return_signature;
+};
+
+class signal
+{
+ public:
+ std::string name;
+ const char* signature;
};
template <typename T>
@@ -134,22 +145,16 @@
}
template <typename CallbackType>
-class callback_method_instance : public callback
+class callback_method_instance
{
- public:
- callback_method_instance(CallbackType&& func) : func_(std::move(func)) {}
- int call(message_t& m) override
- {
- return expandCall(m);
- }
-
private:
using CallbackSignature = boost::callable_traits::args_t<CallbackType>;
using InputTupleType = utility::decay_tuple_t<CallbackSignature>;
+
+ public:
CallbackType func_;
- // optional message-first-argument callback
- int expandCall(message_t& m)
+ int operator()(message_t& m)
{
using DbusTupleType = utility::strip_first_n_args_t<
details::NonDbusArgsCount<InputTupleType>::size(), InputTupleType>;
@@ -160,38 +165,34 @@
return -EINVAL;
}
auto ret = m.new_method_return();
- std::optional<InputTupleType> inputArgs;
if constexpr (callbackWantsMessage<CallbackType>)
{
- inputArgs.emplace(
- std::tuple_cat(std::forward_as_tuple(std::move(m)), dbusArgs));
+ InputTupleType inputArgs =
+ std::tuple_cat(std::forward_as_tuple(std::move(m)), dbusArgs);
+ callFunction(ret, inputArgs, func_);
}
else
{
- inputArgs.emplace(dbusArgs);
+ callFunction(ret, dbusArgs, func_);
}
- callFunction(ret, *inputArgs, func_);
ret.method_return();
return 1;
}
};
template <typename CallbackType>
-class coroutine_method_instance : public callback
+class coroutine_method_instance
{
public:
using self_t = coroutine_method_instance<CallbackType>;
+ boost::asio::io_context& io_;
+ CallbackType func_;
- coroutine_method_instance(boost::asio::io_context& io,
- CallbackType&& func) :
- io_(io),
- func_(std::move(func))
- {}
-
- int call(message_t& m) override
+ int operator()(message_t& m)
{
// make a copy of m to move into the coroutine
message_t b{m};
+
// spawn off a new coroutine to handle the method call
boost::asio::spawn(io_, std::bind_front(&self_t::after_spawn, this, b));
return 1;
@@ -200,131 +201,116 @@
private:
void after_spawn(message_t b, boost::asio::yield_context yield)
{
- std::optional<message_t> err{};
-
- try
- {
- expandCall(yield, b);
- }
- catch (const sdbusplus::exception::SdBusError& e)
- {
- // Catch D-Bus error explicitly called by method handler
- err = b.new_method_errno(e.get_errno(), e.get_error());
- }
- catch (const sdbusplus::exception_t& e)
- {
- err = b.new_method_error(e);
- }
- catch (...)
- {
- err = b.new_method_errno(-EIO);
- }
-
- if (err)
- {
- err->method_return();
- }
- }
-
- using CallbackSignature = boost::callable_traits::args_t<CallbackType>;
- using InputTupleType = utility::decay_tuple_t<CallbackSignature>;
- boost::asio::io_context& io_;
- CallbackType func_;
-
- // co-routine body for call
- void expandCall(boost::asio::yield_context yield, message_t& m)
- {
+ using CallbackSignature = boost::callable_traits::args_t<CallbackType>;
+ using InputTupleType = utility::decay_tuple_t<CallbackSignature>;
using DbusTupleType = utility::strip_first_n_args_t<
details::NonDbusArgsCount<InputTupleType>::size(), InputTupleType>;
DbusTupleType dbusArgs;
try
{
- utility::read_into_tuple(dbusArgs, m);
+ utility::read_into_tuple(dbusArgs, b);
}
catch (const exception::SdBusError& e)
{
- auto ret = m.new_method_errno(e.get_errno(), e.get_error());
+ auto ret = b.new_method_errno(e.get_errno(), e.get_error());
ret.method_return();
return;
}
- auto ret = m.new_method_return();
- std::optional<InputTupleType> inputArgs;
- if constexpr (callbackWantsMessage<CallbackType>)
+ try
{
- inputArgs.emplace(
- std::tuple_cat(std::forward_as_tuple(std::move(yield)),
- std::forward_as_tuple(std::move(m)), dbusArgs));
+ auto ret = b.new_method_return();
+ if constexpr (callbackWantsMessage<CallbackType>)
+ {
+ InputTupleType inputArgs = std::tuple_cat(
+ std::forward_as_tuple(std::move(yield)),
+ std::forward_as_tuple(std::move(b)), dbusArgs);
+ callFunction(ret, inputArgs, func_);
+ }
+ else
+ {
+ InputTupleType inputArgs = std::tuple_cat(
+ std::forward_as_tuple(std::move(yield)), dbusArgs);
+ callFunction(ret, inputArgs, func_);
+ }
+ ret.method_return();
}
- else
+ catch (const sdbusplus::exception::SdBusError& e)
{
- inputArgs.emplace(std::tuple_cat(
- std::forward_as_tuple(std::move(yield)), dbusArgs));
+ // Catch D-Bus error explicitly called by method handler
+ message_t err = b.new_method_errno(e.get_errno(), e.get_error());
+ err.method_return();
}
- callFunction(ret, *inputArgs, func_);
- ret.method_return();
+ catch (const sdbusplus::exception_t& e)
+ {
+ message_t err = b.new_method_error(e);
+ err.method_return();
+ }
+ catch (...)
+ {
+ message_t err = b.new_method_errno(-EIO);
+ err.method_return();
+ }
}
};
template <typename PropertyType, typename CallbackType>
-class callback_get_instance : public callback
+struct callback_get_instance
{
- public:
- callback_get_instance(const std::shared_ptr<PropertyType>& value,
- CallbackType&& func) :
- value_(value),
- func_(std::move(func))
- {}
- int call(message_t& m) override
+ int operator()(message_t& m)
{
*value_ = func_(*value_);
m.append(*value_);
return 1;
}
- private:
std::shared_ptr<PropertyType> value_;
CallbackType func_;
};
-template <typename PropertyType, typename CallbackType>
-class callback_set_instance : public callback_set
+template <typename PropertyType>
+struct callback_set_message_instance
{
- public:
- callback_set_instance(const std::shared_ptr<PropertyType>& value,
- CallbackType&& func) :
- value_(value),
- func_(std::move(func))
- {}
- SetPropertyReturnValue call(message_t& m) override
+ SetPropertyReturnValue operator()(message_t& m)
{
PropertyType input;
m.read(input);
- return set_(input);
- }
- SetPropertyReturnValue set(const std::any& value) override
- {
- return set_(std::any_cast<PropertyType>(value));
- }
-
- private:
- SetPropertyReturnValue set_(const PropertyType& newValue)
- {
PropertyType oldValue = *value_;
- if (func_(newValue, *value_))
+ if (!func_(input, *value_))
{
- if (oldValue == *value_)
- {
- return SetPropertyReturnValue::sameValueUpdated;
- }
- return SetPropertyReturnValue::valueUpdated;
+ return SetPropertyReturnValue::fail;
}
- return SetPropertyReturnValue::fail;
+ if (oldValue == *value_)
+ {
+ return SetPropertyReturnValue::sameValueUpdated;
+ }
+ return SetPropertyReturnValue::valueUpdated;
}
- private:
std::shared_ptr<PropertyType> value_;
- CallbackType func_;
+ std::function<bool(const PropertyType&, PropertyType&)> func_;
+};
+
+template <typename PropertyType>
+struct callback_set_value_instance
+{
+ SetPropertyReturnValue operator()(const std::any& value)
+ {
+ const PropertyType& newValue = std::any_cast<PropertyType>(value);
+ PropertyType oldValue = *value_;
+ if (func_(newValue, *value_) == false)
+ {
+ return SetPropertyReturnValue::fail;
+ }
+ if (oldValue == *value_)
+ {
+ return SetPropertyReturnValue::sameValueUpdated;
+ }
+ return SetPropertyReturnValue::valueUpdated;
+ }
+
+ std::shared_ptr<PropertyType> value_;
+ std::function<bool(const PropertyType&, PropertyType&)> func_;
};
enum class PropertyPermission
@@ -341,9 +327,7 @@
conn_(conn),
path_(path), name_(name)
- {
- vtable_.emplace_back(vtable::start());
- }
+ {}
~dbus_interface()
{
conn_->emit_interfaces_removed(path_.c_str(),
@@ -368,19 +352,17 @@
static const auto type =
utility::tuple_to_array(message::types::type_id<PropertyType>());
- auto nameItr = propertyNames_.emplace(propertyNames_.end(), name);
auto propertyPtr = std::make_shared<PropertyType>(property);
- callbacksGet_[name] = std::make_unique<
- callback_get_instance<PropertyType, CallbackTypeGet>>(
- propertyPtr, std::move(getFunction));
- callbacksSet_[name] = std::make_unique<callback_set_instance<
- PropertyType,
- std::function<int(const PropertyType&, PropertyType&)>>>(
- propertyPtr, details::nop_set_value<PropertyType>);
-
- vtable_.emplace_back(vtable::property(nameItr->c_str(), type.data(),
- get_handler, flags));
+ property_callbacks_.emplace_back(
+ name,
+ callback_get_instance<PropertyType, CallbackTypeGet>(
+ propertyPtr, std::move(getFunction)),
+ callback_set_message_instance<PropertyType>(
+ propertyPtr, details::nop_set_value<PropertyType>),
+ callback_set_value_instance<PropertyType>(
+ propertyPtr, details::nop_set_value<PropertyType>),
+ type.data(), flags);
return true;
}
@@ -414,18 +396,18 @@
static const auto type =
utility::tuple_to_array(message::types::type_id<PropertyType>());
- auto nameItr = propertyNames_.emplace(propertyNames_.end(), name);
auto propertyPtr = std::make_shared<PropertyType>(property);
- callbacksGet_[name] = std::make_unique<
- callback_get_instance<PropertyType, CallbackTypeGet>>(
- propertyPtr, std::move(getFunction));
- callbacksSet_[name] = std::make_unique<
- callback_set_instance<PropertyType, CallbackTypeSet>>(
- propertyPtr, std::move(setFunction));
+ property_callbacks_.emplace_back(
+ name,
+ callback_get_instance<PropertyType, CallbackTypeGet>(
+ propertyPtr, std::move(getFunction)),
+ callback_set_message_instance<PropertyType>(propertyPtr,
+ std::move(setFunction)),
+ callback_set_value_instance<PropertyType>(propertyPtr,
+ std::move(setFunction)),
- vtable_.emplace_back(vtable::property(nameItr->c_str(), type.data(),
- get_handler, set_handler, flags));
+ type.data(), flags);
return true;
}
@@ -497,10 +479,12 @@
{
return false;
}
- auto func = callbacksSet_.find(name);
- if (func != callbacksSet_.end())
+ auto func = std::find_if(
+ property_callbacks_.begin(), property_callbacks_.end(),
+ [&name](const auto& element) { return element.name == name; });
+ if (func != property_callbacks_.end())
{
- SetPropertyReturnValue status = func->second->set(value);
+ SetPropertyReturnValue status = func->on_set_value(value);
if ((status == SetPropertyReturnValue::valueUpdated) ||
(status == SetPropertyReturnValue::sameValueUpdated))
{
@@ -533,8 +517,7 @@
static constexpr auto signature = utility::tuple_to_array(
message::types::type_id<SignalSignature...>());
- const std::string& itr = methodOrSignalNames_.emplace_back(name);
- vtable_.emplace_back(vtable::signal(itr.c_str(), signature.data()));
+ signals_.emplace_back(name, signature);
return true;
}
@@ -557,23 +540,19 @@
static const auto resultType =
utility::tuple_to_array(message::types::type_id<ResultType>());
- const std::string& nameItr = methodOrSignalNames_.emplace_back(name);
-
+ std::function<int(message_t&)> func;
if constexpr (FirstArgIsYield_v<CallbackType>)
{
- callbacksMethod_[name] =
- std::make_unique<coroutine_method_instance<CallbackType>>(
- conn_->get_io_context(), std::move(handler));
+ func = coroutine_method_instance<CallbackType>(
+ conn_->get_io_context(), std::move(handler));
}
else
{
- callbacksMethod_[name] =
- std::make_unique<callback_method_instance<CallbackType>>(
- std::move(handler));
+ func = callback_method_instance<CallbackType>(std::move(handler));
}
+ method_callbacks_.emplace_back(name, std::move(func), argType.data(),
+ resultType.data());
- vtable_.emplace_back(vtable::method(nameItr.c_str(), argType.data(),
- resultType.data(), method_handler));
return true;
}
@@ -583,15 +562,19 @@
sd_bus_error* error)
{
dbus_interface* data = static_cast<dbus_interface*>(userdata);
- auto func = data->callbacksGet_.find(property);
+ auto func = std::find_if(data->property_callbacks_.begin(),
+ data->property_callbacks_.end(),
+ [&property](const auto& element) {
+ return element.name == property;
+ });
auto mesg = message_t(reply);
- if (func != data->callbacksGet_.end())
+ if (func != data->property_callbacks_.end())
{
#ifdef __EXCEPTIONS
try
{
#endif
- return func->second->call(mesg);
+ return func->on_get(mesg);
#ifdef __EXCEPTIONS
}
@@ -615,15 +598,19 @@
sd_bus_error* error)
{
dbus_interface* data = static_cast<dbus_interface*>(userdata);
- auto func = data->callbacksSet_.find(property);
+ auto func = std::find_if(data->property_callbacks_.begin(),
+ data->property_callbacks_.end(),
+ [property](const auto& element) {
+ return element.name == property;
+ });
auto mesg = message_t(value);
- if (func != data->callbacksSet_.end())
+ if (func != data->property_callbacks_.end())
{
#ifdef __EXCEPTIONS
try
{
#endif
- SetPropertyReturnValue status = func->second->call(mesg);
+ SetPropertyReturnValue status = func->on_set_message(mesg);
if ((status == SetPropertyReturnValue::valueUpdated) ||
(status == SetPropertyReturnValue::sameValueUpdated))
{
@@ -657,14 +644,17 @@
{
dbus_interface* data = static_cast<dbus_interface*>(userdata);
auto mesg = message_t(m);
- auto func = data->callbacksMethod_.find(mesg.get_member());
- if (func != data->callbacksMethod_.end())
+ auto member = mesg.get_member();
+ auto func = std::find_if(
+ data->method_callbacks_.begin(), data->method_callbacks_.end(),
+ [&member](const auto& element) { return element.name == member; });
+ if (func != data->method_callbacks_.end())
{
#ifdef __EXCEPTIONS
try
{
#endif
- int status = func->second->call(mesg);
+ int status = func->call(mesg);
if (status == 1)
{
return status;
@@ -706,7 +696,32 @@
{
return false;
}
+ vtable_.reserve(2 + property_callbacks_.size() +
+ method_callbacks_.size() + signals_.size());
+ vtable_.emplace_back(vtable::start());
+ property_callbacks_.shrink_to_fit();
+ for (const auto& element : property_callbacks_)
+ {
+ vtable_.emplace_back(
+ vtable::property(element.name.c_str(), element.signature,
+ get_handler, set_handler, element.flags));
+ }
+ method_callbacks_.shrink_to_fit();
+ for (const auto& element : method_callbacks_)
+ {
+ vtable_.emplace_back(
+ vtable::method(element.name.c_str(), element.arg_signature,
+ element.return_signature, method_handler));
+ }
+ signals_.shrink_to_fit();
+ for (const auto& element : signals_)
+ {
+ vtable_.emplace_back(
+ vtable::signal(element.name.c_str(), element.signature));
+ }
+
vtable_.emplace_back(vtable::end());
+ vtable_.shrink_to_fit();
interface_.emplace(static_cast<sdbusplus::bus_t&>(*conn_),
path_.c_str(), name_.c_str(),
@@ -716,9 +731,9 @@
std::vector<std::string>{name_});
if (!skipPropertyChangedSignal)
{
- for (const std::string& name : propertyNames_)
+ for (const auto& element : property_callbacks_)
{
- signal_property(name);
+ signal_property(element.name);
}
}
return true;
@@ -753,12 +768,11 @@
std::shared_ptr<sdbusplus::asio::connection> conn_;
std::string path_;
std::string name_;
- std::list<std::string> propertyNames_;
- std::list<std::string> methodOrSignalNames_;
- std::unordered_map<std::string, std::unique_ptr<callback>> callbacksGet_;
- std::unordered_map<std::string, std::unique_ptr<callback_set>>
- callbacksSet_;
- std::unordered_map<std::string, std::unique_ptr<callback>> callbacksMethod_;
+
+ std::vector<signal> signals_;
+ std::vector<property_callback> property_callbacks_;
+ std::vector<method_callback> method_callbacks_;
+
std::vector<sd_bus_vtable> vtable_;
std::optional<sdbusplus::server::interface_t> interface_;
};