Fix massive performance regression in attribute table

Follow up 56f7812 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 b4757da)
qgis · Jun 9, 2021 · 4970c3a · 4970c3a
1 parent dcd016b
commit 4970c3a
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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;