core: add nm_settings_get_connection_sorted_by_autoconnect_priority()

Turns out, we call nm_settings_get_connection_clone() *a lot* with sort order
nm_settings_connection_cmp_autoconnect_priority_p_with_data().

As we cache the (differently sorted) list of connections, also cache
the presorted list. The only complication is that every time we still
need to check whether the list is still sorted, because it would be
more complicated to invalidate the list when an entry changes which
affects the sort order. Still, such a check is usually successful
and requires "only" N-1 comparisons.
This commit is contained in:
Thomas Haller 2021-06-15 00:41:45 +02:00
parent 1f09e13f43
commit e7b5650eff
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 115 additions and 65 deletions

View file

@ -1999,16 +1999,16 @@ nm_manager_remove_device(NMManager *self, const char *ifname, NMDeviceType devic
static NMDevice *
system_create_virtual_device(NMManager *self, NMConnection *connection)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self);
NMDeviceFactory * factory;
gs_free NMSettingsConnection **connections = NULL;
guint i;
gs_free char * iface = NULL;
const char * parent_spec;
NMDevice * device = NULL, *parent = NULL;
NMDevice * dev_candidate;
GError * error = NULL;
NMLogLevel log_level;
NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self);
NMDeviceFactory * factory;
NMSettingsConnection *const *connections;
guint i;
gs_free char * iface = NULL;
const char * parent_spec;
NMDevice * device = NULL, *parent = NULL;
NMDevice * dev_candidate;
GError * error = NULL;
NMLogLevel log_level;
g_return_val_if_fail(NM_IS_MANAGER(self), NULL);
g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL);
@ -2091,13 +2091,7 @@ system_create_virtual_device(NMManager *self, NMConnection *connection)
}
/* Create backing resources if the device has any autoconnect connections */
connections = nm_settings_get_connections_clone(
priv->settings,
NULL,
NULL,
NULL,
nm_settings_connection_cmp_autoconnect_priority_p_with_data,
NULL);
connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL);
for (i = 0; connections[i]; i++) {
NMConnection * candidate = nm_settings_connection_get_connection(connections[i]);
NMSettingConnection *s_con;
@ -2136,19 +2130,13 @@ system_create_virtual_device(NMManager *self, NMConnection *connection)
static void
retry_connections_for_parent_device(NMManager *self, NMDevice *device)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self);
gs_free NMSettingsConnection **connections = NULL;
guint i;
NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self);
NMSettingsConnection *const *connections;
guint i;
g_return_if_fail(device);
connections = nm_settings_get_connections_clone(
priv->settings,
NULL,
NULL,
NULL,
nm_settings_connection_cmp_autoconnect_priority_p_with_data,
NULL);
connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL);
for (i = 0; connections[i]; i++) {
NMSettingsConnection *sett_conn = connections[i];
NMConnection * connection = nm_settings_connection_get_connection(sett_conn);
@ -4425,13 +4413,13 @@ find_slaves(NMManager * manager,
guint * out_n_slaves,
gboolean for_user_request)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(manager);
gs_free NMSettingsConnection **all_connections = NULL;
guint n_all_connections;
guint i;
SlaveConnectionInfo * slaves = NULL;
guint n_slaves = 0;
NMSettingConnection * s_con;
NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(manager);
NMSettingsConnection *const *all_connections = NULL;
guint n_all_connections;
guint i;
SlaveConnectionInfo * slaves = NULL;
guint n_slaves = 0;
NMSettingConnection * s_con;
gs_unref_hashtable GHashTable *devices = NULL;
nm_assert(out_n_slaves);
@ -4445,13 +4433,9 @@ find_slaves(NMManager * manager,
* even if a slave was already active, it might be deactivated during
* master reactivation.
*/
all_connections = nm_settings_get_connections_clone(
priv->settings,
&n_all_connections,
NULL,
NULL,
nm_settings_connection_cmp_autoconnect_priority_p_with_data,
NULL);
all_connections =
nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings,
&n_all_connections);
for (i = 0; i < n_all_connections; i++) {
NMSettingsConnection *master_connection = NULL;
NMDevice * master_device = NULL, *slave_device;
@ -6918,9 +6902,9 @@ devices_inited_cb(gpointer user_data)
gboolean
nm_manager_start(NMManager *self, GError **error)
{
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self);
gs_free NMSettingsConnection **connections = NULL;
guint i;
NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self);
NMSettingsConnection *const *connections;
guint i;
nm_device_factory_manager_load_factories(_register_device_factory, self);
@ -6978,13 +6962,7 @@ nm_manager_start(NMManager *self, GError **error)
NM_SETTINGS_SIGNAL_CONNECTION_UPDATED,
G_CALLBACK(connection_updated_cb),
self);
connections = nm_settings_get_connections_clone(
priv->settings,
NULL,
NULL,
NULL,
nm_settings_connection_cmp_autoconnect_priority_p_with_data,
NULL);
connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL);
for (i = 0; connections[i]; i++)
connection_changed(self, connections[i]);

View file

@ -371,6 +371,7 @@ typedef struct {
CList connections_lst_head;
NMSettingsConnection **connections_cached_list;
NMSettingsConnection **connections_cached_list_sorted_by_autoconnect_priority;
GSList *unmanaged_specs;
GSList *unrecognized_specs;
@ -2873,22 +2874,37 @@ impl_settings_reload_connections(NMDBusObject * obj,
static void
_clear_connections_cached_list(NMSettingsPrivate *priv)
{
if (!priv->connections_cached_list)
return;
nm_assert(priv->connections_len == NM_PTRARRAY_LEN(priv->connections_cached_list));
if (priv->connections_cached_list) {
nm_assert(priv->connections_len == NM_PTRARRAY_LEN(priv->connections_cached_list));
#if NM_MORE_ASSERTS
/* set the pointer to a bogus value. This makes it more apparent
* if somebody has a reference to the cached list and still uses
* it. That is a bug, this code just tries to make it blow up
* more eagerly. */
memset(priv->connections_cached_list,
0x43,
sizeof(NMSettingsConnection *) * (priv->connections_len + 1));
/* set the pointer to a bogus value. This makes it more apparent
* if somebody has a reference to the cached list and still uses
* it. That is a bug, this code just tries to make it blow up
* more eagerly. */
memset(priv->connections_cached_list,
0x43,
sizeof(NMSettingsConnection *) * (priv->connections_len + 1));
#endif
nm_clear_g_free(&priv->connections_cached_list);
nm_clear_g_free(&priv->connections_cached_list);
}
if (priv->connections_cached_list_sorted_by_autoconnect_priority) {
nm_assert(priv->connections_len
== NM_PTRARRAY_LEN(priv->connections_cached_list_sorted_by_autoconnect_priority));
#if NM_MORE_ASSERTS
/* set the pointer to a bogus value. This makes it more apparent
* if somebody has a reference to the cached list and still uses
* it. That is a bug, this code just tries to make it blow up
* more eagerly. */
memset(priv->connections_cached_list_sorted_by_autoconnect_priority,
0x42,
sizeof(NMSettingsConnection *) * (priv->connections_len + 1));
#endif
nm_clear_g_free(&priv->connections_cached_list_sorted_by_autoconnect_priority);
}
}
static void
@ -3027,6 +3043,55 @@ nm_settings_get_connections(NMSettings *self, guint *out_len)
return priv->connections_cached_list;
}
NMSettingsConnection *const *
nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, guint *out_len)
{
NMSettingsPrivate *priv;
gboolean needs_sort = FALSE;
g_return_val_if_fail(NM_IS_SETTINGS(self), NULL);
priv = NM_SETTINGS_GET_PRIVATE(self);
nm_assert(priv->connections_len == c_list_length(&priv->connections_lst_head));
nm_assert(
!priv->connections_cached_list_sorted_by_autoconnect_priority
|| (priv->connections_len
== NM_PTRARRAY_LEN(priv->connections_cached_list_sorted_by_autoconnect_priority)));
if (!priv->connections_cached_list_sorted_by_autoconnect_priority) {
NMSettingsConnection *const *list_cached;
guint len;
list_cached = nm_settings_get_connections(self, &len);
priv->connections_cached_list_sorted_by_autoconnect_priority =
nm_memdup(list_cached, sizeof(NMSettingsConnection *) * (len + 1));
needs_sort = (len > 1);
} else if (!nm_utils_ptrarray_is_sorted(
(gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority,
priv->connections_len,
FALSE,
nm_settings_connection_cmp_autoconnect_priority_with_data,
NULL)) {
/* We cache the sorted list, but we don't monitor all entries whether they
* get modified to invalidate the sort order. So every time we have to check
* whether the sort order is still correct. The vast majority of the time it
* is, and this check is faster than sorting anew. */
needs_sort = TRUE;
}
if (needs_sort) {
g_qsort_with_data(priv->connections_cached_list_sorted_by_autoconnect_priority,
priv->connections_len,
sizeof(NMSettingsConnection *),
nm_settings_connection_cmp_autoconnect_priority_p_with_data,
NULL);
}
NM_SET_OUT(out_len, priv->connections_len);
return priv->connections_cached_list_sorted_by_autoconnect_priority;
}
/**
* nm_settings_get_connections_clone:
* @self: the #NMSetting
@ -3058,9 +3123,13 @@ nm_settings_get_connections_clone(NMSettings * self,
g_return_val_if_fail(NM_IS_SETTINGS(self), NULL);
list_cached = nm_settings_get_connections(self, &len);
if (sort_compare_func == nm_settings_connection_cmp_autoconnect_priority_p_with_data) {
list_cached = nm_settings_get_connections_sorted_by_autoconnect_priority(self, &len);
sort_compare_func = NULL;
} else
list_cached = nm_settings_get_connections(self, &len);
#if NM_MORE_ASSERTS
#if NM_MORE_ASSERTS > 10
nm_assert(list_cached);
for (i = 0; i < len; i++)
nm_assert(NM_IS_SETTINGS_CONNECTION(list_cached[i]));

View file

@ -79,6 +79,9 @@ void nm_settings_add_connection_dbus(NMSettings * self,
NMSettingsConnection *const *nm_settings_get_connections(NMSettings *settings, guint *out_len);
NMSettingsConnection *const *
nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, guint *out_len);
NMSettingsConnection **nm_settings_get_connections_clone(NMSettings * self,
guint * out_len,
NMSettingsConnectionFilterFunc func,