Batch delayed flushes to improve performance on macOS

app/backend/computermanager.cpp

@@ -15,6 +15,9 @@
 #define SER_HOSTS "hosts"
 #define SER_HOSTS_BACKUP "hostsbackup"
 
+// We will wait up to this length of time to batch delayed flushes
+#define DELAYED_FLUSH_TIMEOUT_MS 10000
+
 class PcMonitorThread : public QThread
 {
     Q_OBJECT
@@ -149,7 +152,7 @@ ComputerManager::ComputerManager(QObject *parent)
       m_PollingRef(0),
       m_MdnsBrowser(nullptr),
       m_CompatFetcher(nullptr),
-      m_NeedsDelayedFlush(false)
+      m_NeedsFlush(FlushType::None)
 {
     QSettings settings;
 
@@ -190,16 +193,17 @@ ComputerManager::~ComputerManager()
     // Stop the delayed flush thread before acquiring the lock in write mode
     // to avoid deadlocking with a flush that needs the lock in read mode.
     {
-        // Wake the delayed flush thread
+        // Wake the delayed flush thread for an immediate flush
         m_DelayedFlushThread->requestInterruption();
         m_DelayedFlushCondition.wakeOne();
+        m_ImmediateFlushCondition.wakeOne();
 
         // Wait for it to terminate (and finish any pending flush)
         m_DelayedFlushThread->wait();
         delete m_DelayedFlushThread;
 
         // Delayed flushes should have completed by now
-        Q_ASSERT(!m_NeedsDelayedFlush);
+        Q_ASSERT(m_NeedsFlush == FlushType::None);
     }
 
     QWriteLocker lock(&m_Lock);
@@ -237,19 +241,25 @@ void DelayedFlushThread::run() {
         {
             QMutexLocker locker(&m_ComputerManager->m_DelayedFlushMutex);
 
-            while (!QThread::currentThread()->isInterruptionRequested() && !m_ComputerManager->m_NeedsDelayedFlush) {
+            // Wait for any flush to be pending
+            while (!QThread::currentThread()->isInterruptionRequested() && m_ComputerManager->m_NeedsFlush == ComputerManager::FlushType::None) {
                 m_ComputerManager->m_DelayedFlushCondition.wait(&m_ComputerManager->m_DelayedFlushMutex);
             }
 
             // Bail without flushing if we woke up for an interruption alone.
             // If we have both an interruption and a flush request, do the flush.
-            if (!m_ComputerManager->m_NeedsDelayedFlush) {
+            if (m_ComputerManager->m_NeedsFlush == ComputerManager::FlushType::None) {
                 Q_ASSERT(QThread::currentThread()->isInterruptionRequested());
                 break;
             }
 
-            // Reset the delayed flush flag to ensure any racing saveHosts() call will set it again
+            // If this is a delayed flush and we're not being interrupted, wait on the immediate flush condition (up to the delayed flush timeout).
-            m_ComputerManager->m_NeedsDelayedFlush = false;
+            while (!QThread::currentThread()->isInterruptionRequested() &&
+                   m_ComputerManager->m_NeedsFlush != ComputerManager::FlushType::Immediate &&
+                   m_ComputerManager->m_ImmediateFlushCondition.wait(&m_ComputerManager->m_DelayedFlushMutex, DELAYED_FLUSH_TIMEOUT_MS));
+
+            // Reset the flush condition
+            m_ComputerManager->m_NeedsFlush = ComputerManager::FlushType::None;
         }
 
         // Perform the flush
@@ -287,15 +297,24 @@ void DelayedFlushThread::run() {
     }
 }
 
-void ComputerManager::saveHosts()
+void ComputerManager::saveHosts(bool immediate)
 {
     Q_ASSERT(m_DelayedFlushThread != nullptr && m_DelayedFlushThread->isRunning());
 
     // Punt to a worker thread because QSettings on macOS can take ages (> 500 ms)
     // to persist our host list to disk (especially when a host has a bunch of apps).
     QMutexLocker locker(&m_DelayedFlushMutex);
-    m_NeedsDelayedFlush = true;
+    if (m_NeedsFlush == FlushType::None) {
+        m_NeedsFlush = immediate ? FlushType::Immediate : FlushType::Delayed;
+    }
+    else if (m_NeedsFlush == FlushType::Delayed && immediate) {
+        // Upgrade a delayed flush to an immediate flush if an immediate request comes in
+        m_NeedsFlush = FlushType::Immediate;
+    }
     m_DelayedFlushCondition.wakeOne();
+    if (immediate) {
+        m_ImmediateFlushCondition.wakeOne();
+    }
 }
 
 QHostAddress ComputerManager::getBestGlobalAddressV6(QVector<QHostAddress> &addresses)
@@ -442,8 +461,8 @@ void ComputerManager::handleComputerStateChanged(NvComputer* computer)
         emit quitAppCompleted(QVariant());
     }
 
-    // Save updated hosts to QSettings
+    // Save updated hosts to QSettings (delayed and batchable)
-    saveHosts();
+    saveHosts(false);
 }
 
 QVector<NvComputer*> ComputerManager::getComputers()
@@ -479,8 +498,8 @@ public:
             m_ComputerManager->m_KnownHosts.remove(m_Computer->uuid);
         }
 
-        // Persist the new host list
+        // Persist the new host list immediately
-        m_ComputerManager->saveHosts();
+        m_ComputerManager->saveHosts(true);
 
         // Delete the polling entry first. This will stop all polling threads too.
         delete pollingEntry;
@@ -520,9 +539,6 @@ void ComputerManager::renameHost(NvComputer* computer, QString name)
 
 void ComputerManager::clientSideAttributeUpdated(NvComputer* computer)
 {
-    // Persist the change
-    saveHosts();
-
     // Notify the UI of the state change
     handleComputerStateChanged(computer);
 }
@@ -579,8 +595,9 @@ private:
                emit pairingCompleted(m_Computer, tr("Another pairing attempt is already in progress."));
                break;
            case NvPairingManager::PairState::PAIRED:
-               // Persist the newly pinned server certificate for this host
+               // Persist the newly pinned server certificate for this host. We do this immediately because
-               m_ComputerManager->saveHosts();
+               // loss of the pinned cert invalidates the pairing. This is also a very infrequent operation.
+               m_ComputerManager->saveHosts(true);
 
                emit pairingCompleted(m_Computer, nullptr);
                break;

app/backend/computermanager.h

@@ -249,7 +249,7 @@ private slots:
     void handleMdnsServiceResolved(MdnsPendingComputer* computer, QVector<QHostAddress>& addresses);
 
 private:
-    void saveHosts();
+    void saveHosts(bool immediate);
 
     QHostAddress getBestGlobalAddressV6(QVector<QHostAddress>& addresses);
 
@@ -266,5 +266,10 @@ private:
     DelayedFlushThread* m_DelayedFlushThread;
     QMutex m_DelayedFlushMutex;
     QWaitCondition m_DelayedFlushCondition;
-    bool m_NeedsDelayedFlush;
+    QWaitCondition m_ImmediateFlushCondition;
+    enum class FlushType {
+        None,
+        Delayed,
+        Immediate
+    } m_NeedsFlush;
 };