From 4970c3a9dbc66d1b2d155e3cdd07df200ee1c14a Mon Sep 17 00:00:00 2001
From: Nyall Dawson <nyall.dawson@gmail.com>
Date: Tue, 8 Jun 2021 13:31:56 +1000
Subject: [PATCH] Fix massive performance regression in attribute table

Follow up 56f7812ca1e

This commit fixed the ordering of features coming from the
vector layer cache for the attribute table, but came with a massive
speed impact due to the repeated calls QList::contains for
every feature fetched. For any moderately sized table or above
these calls stacked up into multiple minute delays in opening
the table.

Avoid this by tracking the added feature ids in a separate
unordered set, so that we don't need to check through the
ordered list for existing features at all.

Eg a 500k feature gpkg was taking 10 minutes to open the table.
With this optimization that's back down to 20 seconds.

(cherry picked from commit b4757dacc679914441920c739084de412281eb27)
---
 .../auto_generated/qgsvectorlayercache.sip.in |  1 -
 src/core/qgsvectorlayercache.cpp              | 23 +++++++++++++++++--
 src/core/qgsvectorlayercache.h                | 13 +++++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/python/core/auto_generated/qgsvectorlayercache.sip.in b/python/core/auto_generated/qgsvectorlayercache.sip.in
index 4049ccf1fa72..5f4d41c5479c 100644
--- a/python/core/auto_generated/qgsvectorlayercache.sip.in
+++ b/python/core/auto_generated/qgsvectorlayercache.sip.in
@@ -11,7 +11,6 @@
 
 
 
-
 class QgsVectorLayerCache : QObject
 {
 %Docstring
diff --git a/src/core/qgsvectorlayercache.cpp b/src/core/qgsvectorlayercache.cpp
index 0dc790dec76c..43021874b317 100644
--- a/src/core/qgsvectorlayercache.cpp
+++ b/src/core/qgsvectorlayercache.cpp
@@ -175,7 +175,17 @@ bool QgsVectorLayerCache::removeCachedFeature( QgsFeatureId fid )
 {
   bool removed = mCache.remove( fid );
   if ( removed )
-    mCacheOrderedKeys.removeOne( fid );
+  {
+    auto unorderedIt = std::find( mCacheUnorderedKeys.begin(), mCacheUnorderedKeys.end(), fid );
+    if ( unorderedIt != mCacheUnorderedKeys.end() )
+    {
+      mCacheUnorderedKeys.erase( unorderedIt );
+
+      auto orderedIt = std::find( mCacheOrderedKeys.begin(), mCacheOrderedKeys.end(), fid );
+      if ( orderedIt != mCacheOrderedKeys.end() )
+        mCacheOrderedKeys.erase( orderedIt );
+    }
+  }
   return removed;
 }
 
@@ -268,7 +278,15 @@ void QgsVectorLayerCache::onJoinAttributeValueChanged( QgsFeatureId fid, int fie
 void QgsVectorLayerCache::featureDeleted( QgsFeatureId fid )
 {
   mCache.remove( fid );
-  mCacheOrderedKeys.removeOne( fid );
+
+  auto it = mCacheUnorderedKeys.find( fid );
+  if ( it != mCacheUnorderedKeys.end() )
+  {
+    mCacheUnorderedKeys.erase( it );
+    auto orderedIt = std::find( mCacheOrderedKeys.begin(), mCacheOrderedKeys.end(), fid );
+    if ( orderedIt != mCacheOrderedKeys.end() )
+      mCacheOrderedKeys.erase( orderedIt );
+  }
 }
 
 void QgsVectorLayerCache::onFeatureAdded( QgsFeatureId fid )
@@ -328,6 +346,7 @@ void QgsVectorLayerCache::invalidate()
 {
   mCache.clear();
   mCacheOrderedKeys.clear();
+  mCacheUnorderedKeys.clear();
   mFullCache = false;
   emit invalidated();
 }
diff --git a/src/core/qgsvectorlayercache.h b/src/core/qgsvectorlayercache.h
index 19981f5b9d4f..a2458daff41c 100644
--- a/src/core/qgsvectorlayercache.h
+++ b/src/core/qgsvectorlayercache.h
@@ -24,7 +24,8 @@
 #include "qgsfield.h"
 #include "qgsfeaturerequest.h"
 #include "qgsfeatureiterator.h"
-
+#include <unordered_set>
+#include <deque>
 #include <QCache>
 
 class QgsVectorLayer;
@@ -393,12 +394,20 @@ class CORE_EXPORT QgsVectorLayerCache : public QObject
     {
       QgsCachedFeature *cachedFeature = new QgsCachedFeature( feat, this );
       mCache.insert( feat.id(), cachedFeature );
-      if ( !mCacheOrderedKeys.contains( feat.id() ) )
+      if ( mCacheUnorderedKeys.find( feat.id() ) == mCacheUnorderedKeys.end() )
+      {
+        mCacheUnorderedKeys.insert( feat.id() );
         mCacheOrderedKeys << feat.id();
+      }
     }
 
     QgsVectorLayer *mLayer = nullptr;
     QCache< QgsFeatureId, QgsCachedFeature > mCache;
+
+    // we need two containers here. One is used for efficient tracking of the IDs which have been added to the cache, the other
+    // is used to store the order of the incoming feature ids, so that we can correctly iterate through features in the original order.
+    // the ordered list alone is far too slow to handle this -- searching for existing items in a list is magnitudes slower than the unordered_set
+    std::unordered_set< QgsFeatureId > mCacheUnorderedKeys;
     QList< QgsFeatureId > mCacheOrderedKeys;
 
     bool mCacheGeometry = true;