From 877d2b236f8f5ffd0d09c0955bfa75cbf05fc15f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 17 Jun 2021 19:29:33 +0200
Subject: [PATCH] core: avoid checking sort order for cached settings list

We now have a cached list of NMSettingsConnection instances,
sorted by their autoconnect priority.

However, the sort order nm_settings_connection_cmp_autoconnect_priority()
depends on various properties of the connection:

 - "connection.autoconnect" and "connection.autoconnect-priority"
 - the timestamp
 - "connection.uuid"

These properties almost never change, so it's a waste that every call
to nm_settings_get_connections_sorted_by_autoconnect_priority() needs
to check whether the sort order is still satisfied.

We can do better by tracking when the sort order might have been
destroyed and only check in those (much fewer) cases.

Note that we end up calling nm_settings_get_connections_sorted_by_autoconnect_priority()
a lot, so this makes a difference.
---
 src/core/settings/nm-settings-connection.c |  3 ++
 src/core/settings/nm-settings.c            | 46 ++++++++++++++++------
 src/core/settings/nm-settings.h            |  2 +
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c
index 71431f29d9..641f3297b8 100644
--- a/src/core/settings/nm-settings-connection.c
+++ b/src/core/settings/nm-settings-connection.c
@@ -337,6 +337,7 @@ _nm_settings_connection_set_connection(NMSettingsConnection *           self,
         nmtst_connection_assert_unchanging(priv->connection);
 
         _getsettings_cached_clear(priv);
+        _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(priv->settings);
 
         /* note that we only return @connection_old if the new connection actually differs from
          * before.
@@ -2251,6 +2252,8 @@ nm_settings_connection_update_timestamp(NMSettingsConnection *self, guint64 time
 
     _LOGT("timestamp: set timestamp %" G_GUINT64_FORMAT, timestamp);
 
+    _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(priv->settings);
+
     if (!priv->kf_db_timestamps)
         return;
 
diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c
index bb58af9899..c876ea148c 100644
--- a/src/core/settings/nm-settings.c
+++ b/src/core/settings/nm-settings.c
@@ -390,6 +390,12 @@ typedef struct {
 
     bool started : 1;
 
+    /* Whether NMSettingsConnections changed in a way that affects the comparison
+     * with nm_settings_connection_cmp_autoconnect_priority_with_data(). In that case,
+     * we may need to re-sort the connections_cached_list_sorted_by_autoconnect_priority
+     * list. */
+    bool sorted_by_autoconnect_priority_maybe_changed : 1;
+
 } NMSettingsPrivate;
 
 struct _NMSettings {
@@ -2871,6 +2877,14 @@ impl_settings_reload_connections(NMDBusObject *                     obj,
 
 /*****************************************************************************/
 
+void
+_nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(NMSettings *self)
+{
+    NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE(self);
+
+    priv->sorted_by_autoconnect_priority_maybe_changed = TRUE;
+}
+
 static void
 _clear_connections_cached_list(NMSettingsPrivate *priv)
 {
@@ -3067,19 +3081,29 @@ nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, gui
         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;
+    } else if (priv->sorted_by_autoconnect_priority_maybe_changed) {
+        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;
+        }
+    } else {
+        nm_assert(nm_utils_ptrarray_is_sorted(
+            (gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority,
+            priv->connections_len,
+            TRUE,
+            nm_settings_connection_cmp_autoconnect_priority_with_data,
+            NULL));
     }
 
+    priv->sorted_by_autoconnect_priority_maybe_changed = FALSE;
     if (needs_sort) {
         g_qsort_with_data(priv->connections_cached_list_sorted_by_autoconnect_priority,
                           priv->connections_len,
diff --git a/src/core/settings/nm-settings.h b/src/core/settings/nm-settings.h
index fbbc957477..ce18fecccf 100644
--- a/src/core/settings/nm-settings.h
+++ b/src/core/settings/nm-settings.h
@@ -130,4 +130,6 @@ const char *nm_settings_get_startup_complete_blocked_reason(NMSettings *self,
 
 void nm_settings_kf_db_write(NMSettings *settings);
 
+void _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(NMSettings *self);
+
 #endif /* __NM_SETTINGS_H__ */