core/dbus: cache variants for D-Bus exported properties

GVariant is immutable and can nicely be shared and cached.
Cache the property variants. This makes getting the properties
faster, at the expense of using some extra memory.

Tested with https://tratt.net/laurie/src/multitime/

  $ multitime -n 200 -s 0 bash -c 'echo -n .; exec busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        0.013+/-0.0000      0.001       0.012       0.013       0.019
  # real(after)         0.013+/-0.0000      0.002       0.011       0.012       0.034

  $ multitime -n 100 -s 0 bash -c 'for i in {1..5}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null & done; wait; echo -n .'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        0.040+/-0.0000      0.002       0.037       0.040       0.049
  # real(after)         0.037+/-0.0000      0.002       0.034       0.036       0.045

  $ multitime -n 30 -s 0 bash -c 'for i in {1..100}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects &>/dev/null & done; wait; echo -n .'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        0.704+/-0.0000      0.016       0.687       0.701       0.766
  # real(after)         0.639+/-0.0000      0.013       0.622       0.637       0.687

  $ multitime -n 200 -s 0 bash -c 'echo -n .; exec nmcli &>/dev/null'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        0.092+/-0.0000      0.005       0.081       0.092       0.119
  # real(after)         0.092+/-0.0000      0.005       0.080       0.091       0.123

  $ multitime -n 100 -s 0 bash -c 'for i in {1..5}; do nmcli &>/dev/null & done; wait; echo -n .'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        0.436+/-0.0000      0.043       0.375       0.424       0.600
  # real(after)         0.413+/-0.0000      0.022       0.380       0.410       0.558

  $ multitime -n 20 -s 0 bash -c 'for i in {1..100}; do nmcli &>/dev/null & done; wait; echo -n .'
  #                     Mean                Std.Dev.    Min         Median      Max
  # real(before)        8.796+/-0.1070      0.291       8.073       8.818       9.247
  # real(after)         8.736+/-0.0893      0.243       8.017       8.780       9.101

The time savings are small, but that is because caching mostly speeds up
the GetManagedObjects calls, and that is only a small part of the entire
nmcli call from client side.
This commit is contained in:
Thomas Haller
2018-03-23 20:06:38 +01:00
parent 0ed5cd5442
commit 640736ff65
3 changed files with 65 additions and 24 deletions

View File

@@ -45,12 +45,17 @@
/*****************************************************************************/ /*****************************************************************************/
typedef struct {
GVariant *value;
} PropertyCacheData;
typedef struct { typedef struct {
CList registration_lst; CList registration_lst;
NMDBusObject *obj; NMDBusObject *obj;
NMDBusObjectClass *klass; NMDBusObjectClass *klass;
guint info_idx; guint info_idx;
guint registration_id; guint registration_id;
PropertyCacheData property_cache[];
} RegistrationData; } RegistrationData;
/* we require that @path is the first member of NMDBusManagerData /* we require that @path is the first member of NMDBusManagerData
@@ -807,7 +812,8 @@ dbus_vtable_method_call (GDBusConnection *connection,
nm_assert (nm_streq (property_interface, interface_info->parent.name)); nm_assert (nm_streq (property_interface, interface_info->parent.name));
property_info = (const NMDBusPropertyInfoExtended *) nm_dbus_utils_interface_info_lookup_property (&interface_info->parent, property_info = (const NMDBusPropertyInfoExtended *) nm_dbus_utils_interface_info_lookup_property (&interface_info->parent,
property_name); property_name,
NULL);
if ( !property_info if ( !property_info
|| !NM_FLAGS_HAS (property_info->parent.flags, G_DBUS_PROPERTY_INFO_FLAGS_WRITABLE)) || !NM_FLAGS_HAS (property_info->parent.flags, G_DBUS_PROPERTY_INFO_FLAGS_WRITABLE))
g_return_if_reached (); g_return_if_reached ();
@@ -854,6 +860,33 @@ dbus_vtable_method_call (GDBusConnection *connection,
parameters); parameters);
} }
static GVariant *
_obj_get_property (RegistrationData *reg_data,
guint property_idx,
gboolean refetch)
{
const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
const NMDBusPropertyInfoExtended *property_info;
GVariant *value;
property_info = (const NMDBusPropertyInfoExtended *) (interface_info->parent.properties[property_idx]);
if (refetch)
nm_clear_g_variant (&reg_data->property_cache[property_idx].value);
else {
value = reg_data->property_cache[property_idx].value;
if (value)
goto out;
}
value = nm_dbus_utils_get_property (G_OBJECT (reg_data->obj),
property_info->parent.signature,
property_info->property_name);
reg_data->property_cache[property_idx].value = value;
out:
return g_variant_ref (value);
}
static GVariant * static GVariant *
dbus_vtable_get_property (GDBusConnection *connection, dbus_vtable_get_property (GDBusConnection *connection,
const char *sender, const char *sender,
@@ -865,16 +898,14 @@ dbus_vtable_get_property (GDBusConnection *connection,
{ {
RegistrationData *reg_data = user_data; RegistrationData *reg_data = user_data;
const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data); const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
const NMDBusPropertyInfoExtended *property_info; guint property_idx;
property_info = (const NMDBusPropertyInfoExtended *) nm_dbus_utils_interface_info_lookup_property (&interface_info->parent, if (!nm_dbus_utils_interface_info_lookup_property (&interface_info->parent,
property_name); property_name,
if (!property_info) &property_idx))
g_return_val_if_reached (NULL); g_return_val_if_reached (NULL);
return nm_dbus_utils_get_property (G_OBJECT (reg_data->obj), return _obj_get_property (reg_data, property_idx, FALSE);
property_info->parent.signature,
property_info->property_name);
} }
static const GDBusInterfaceVTable dbus_vtable = { static const GDBusInterfaceVTable dbus_vtable = {
@@ -930,8 +961,9 @@ _obj_register (NMDBusManager *self,
RegistrationData *reg_data; RegistrationData *reg_data;
gs_free_error GError *error = NULL; gs_free_error GError *error = NULL;
guint registration_id; guint registration_id;
guint prop_len = NM_PTRARRAY_LEN (interface_info->parent.properties);
reg_data = g_slice_new (RegistrationData); reg_data = g_malloc0 (sizeof (RegistrationData) + (sizeof (PropertyCacheData) * prop_len));
registration_id = g_dbus_connection_register_object (priv->connection, registration_id = g_dbus_connection_register_object (priv->connection,
obj->internal.path, obj->internal.path,
@@ -997,14 +1029,23 @@ _obj_unregister (NMDBusManager *self,
g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); g_variant_builder_init (&builder, G_VARIANT_TYPE ("as"));
while ((reg_data = c_list_last_entry (&obj->internal.registration_lst_head, RegistrationData, registration_lst))) { while ((reg_data = c_list_last_entry (&obj->internal.registration_lst_head, RegistrationData, registration_lst))) {
const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
guint i;
g_variant_builder_add (&builder, g_variant_builder_add (&builder,
"s", "s",
_reg_data_get_interface_info (reg_data)->parent.name); interface_info->parent.name);
c_list_unlink_stale (&reg_data->registration_lst); c_list_unlink_stale (&reg_data->registration_lst);
if (!g_dbus_connection_unregister_object (priv->connection, reg_data->registration_id)) if (!g_dbus_connection_unregister_object (priv->connection, reg_data->registration_id))
nm_assert_not_reached (); nm_assert_not_reached ();
if (interface_info->parent.properties) {
for (i = 0; interface_info->parent.properties[i]; i++)
nm_clear_g_variant (&reg_data->property_cache[i].value);
}
g_type_class_unref (reg_data->klass); g_type_class_unref (reg_data->klass);
g_slice_free (RegistrationData, reg_data); g_free (reg_data);
} }
g_dbus_connection_emit_signal (priv->connection, g_dbus_connection_emit_signal (priv->connection,
@@ -1142,9 +1183,7 @@ _nm_dbus_manager_obj_notify (NMDBusObject *obj,
if (!nm_streq (property_info->property_name, pspec->name)) if (!nm_streq (property_info->property_name, pspec->name))
continue; continue;
value = nm_dbus_utils_get_property (G_OBJECT (obj), value = _obj_get_property (reg_data, i, TRUE);
property_info->parent.signature,
property_info->property_name);
if ( property_info->include_in_legacy_property_changed if ( property_info->include_in_legacy_property_changed
&& any_legacy_signals) { && any_legacy_signals) {
@@ -1277,9 +1316,10 @@ _nm_dbus_manager_obj_emit_signal (NMDBusObject *obj,
static GVariantBuilder * static GVariantBuilder *
_obj_collect_properties_per_interface (NMDBusObject *obj, _obj_collect_properties_per_interface (NMDBusObject *obj,
const NMDBusInterfaceInfoExtended *interface_info, RegistrationData *reg_data,
GVariantBuilder *builder) GVariantBuilder *builder)
{ {
const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
guint i; guint i;
g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sv}"));
@@ -1288,9 +1328,7 @@ _obj_collect_properties_per_interface (NMDBusObject *obj,
const NMDBusPropertyInfoExtended *property_info = (const NMDBusPropertyInfoExtended *) interface_info->parent.properties[i]; const NMDBusPropertyInfoExtended *property_info = (const NMDBusPropertyInfoExtended *) interface_info->parent.properties[i];
gs_unref_variant GVariant *variant = NULL; gs_unref_variant GVariant *variant = NULL;
variant = nm_dbus_utils_get_property (G_OBJECT (obj), variant = _obj_get_property (reg_data, i, FALSE);
property_info->parent.signature,
property_info->property_name);
g_variant_builder_add (builder, g_variant_builder_add (builder,
"{sv}", "{sv}",
property_info->parent.name, property_info->parent.name,
@@ -1309,14 +1347,13 @@ _obj_collect_properties_all (NMDBusObject *obj,
g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sa{sv}}")); g_variant_builder_init (builder, G_VARIANT_TYPE ("a{sa{sv}}"));
c_list_for_each_entry (reg_data, &obj->internal.registration_lst_head, registration_lst) { c_list_for_each_entry (reg_data, &obj->internal.registration_lst_head, registration_lst) {
const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
GVariantBuilder properties_builder; GVariantBuilder properties_builder;
g_variant_builder_add (builder, g_variant_builder_add (builder,
"{sa{sv}}", "{sa{sv}}",
interface_info->parent.name, _reg_data_get_interface_info (reg_data)->parent.name,
_obj_collect_properties_per_interface (obj, _obj_collect_properties_per_interface (obj,
interface_info, reg_data,
&properties_builder)); &properties_builder));
} }

View File

@@ -35,7 +35,8 @@ const GDBusSignalInfo nm_signal_info_property_changed_legacy = NM_DEFINE_GDBUS_S
GDBusPropertyInfo * GDBusPropertyInfo *
nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info, nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info,
const char *property_name) const char *property_name,
guint *property_idx)
{ {
guint i; guint i;
@@ -48,8 +49,10 @@ nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interfac
for (i = 0; interface_info->properties[i]; i++) { for (i = 0; interface_info->properties[i]; i++) {
GDBusPropertyInfo *info = interface_info->properties[i]; GDBusPropertyInfo *info = interface_info->properties[i];
if (nm_streq (info->name, property_name)) if (nm_streq (info->name, property_name)) {
NM_SET_OUT (property_idx, i);
return info; return info;
}
} }
} }

View File

@@ -152,7 +152,8 @@ extern const GDBusSignalInfo nm_signal_info_property_changed_legacy;
/*****************************************************************************/ /*****************************************************************************/
GDBusPropertyInfo *nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info, GDBusPropertyInfo *nm_dbus_utils_interface_info_lookup_property (const GDBusInterfaceInfo *interface_info,
const char *property_name); const char *property_name,
guint *property_idx);
GDBusMethodInfo *nm_dbus_utils_interface_info_lookup_method (const GDBusInterfaceInfo *interface_info, GDBusMethodInfo *nm_dbus_utils_interface_info_lookup_method (const GDBusInterfaceInfo *interface_info,
const char *method_name); const char *method_name);