Merge GenericObjectCache update and remove functions

They are always called together. Single iteration over the items is more
efficient along with locking the mutex only once.

apps/openmw_test_suite/resource/testobjectcache.cpp

@@ -63,9 +63,7 @@ namespace Resource
 
             const double referenceTime = 1000;
             const double expiryDelay = 1;
-
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime, expiryDelay);
             EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
         }
 
@@ -92,16 +90,13 @@ namespace Resource
             const int key = 42;
             cache->addEntryToObjectCache(key, nullptr, referenceTime + expiryDelay);
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
-            cache->removeExpiredObjectsInCache(referenceTime);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + 2 * expiryDelay);
-            cache->removeExpiredObjectsInCache(referenceTime + expiryDelay);
+            cache->update(referenceTime + 2 * expiryDelay, expiryDelay);
             EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
         }
 
@@ -117,12 +112,10 @@ namespace Resource
             cache->addEntryToObjectCache(key, value);
             value = nullptr;
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
-            cache->removeExpiredObjectsInCache(referenceTime);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
         }
 
@@ -137,12 +130,10 @@ namespace Resource
             osg::ref_ptr<Object> value(new Object);
             cache->addEntryToObjectCache(key, value);
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
-            cache->removeExpiredObjectsInCache(referenceTime);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(value));
         }
 
@@ -158,12 +149,10 @@ namespace Resource
             cache->addEntryToObjectCache(key, value);
             value = nullptr;
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            cache->update(referenceTime + expiryDelay / 2, expiryDelay);
             EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
         }
 
@@ -177,12 +166,10 @@ namespace Resource
             const int key = 42;
             cache->addEntryToObjectCache(key, nullptr);
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            cache->update(referenceTime + expiryDelay / 2, expiryDelay);
             EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
         }
 
@@ -196,16 +183,13 @@ namespace Resource
             const int key = 42;
             cache->addEntryToObjectCache(key, nullptr);
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            cache->update(referenceTime, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
-            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            cache->update(referenceTime + expiryDelay / 2, expiryDelay);
             ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
 
-            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
-            cache->removeExpiredObjectsInCache(referenceTime);
+            cache->update(referenceTime + expiryDelay, expiryDelay);
             EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
         }
 

components/resource/objectcache.hpp

@@ -24,6 +24,7 @@
 #include <osg/Referenced>
 #include <osg/ref_ptr>
 
+#include <algorithm>
 #include <map>
 #include <mutex>
 #include <optional>
@@ -48,48 +49,25 @@ namespace Resource
         {
         }
 
-        /** For each object in the cache which has an reference count greater than 1
-         * (and therefore referenced by elsewhere in the application) set the time stamp
-         * for that object in the cache to specified time.
-         * This would typically be called once per frame by applications which are doing database paging,
-         * and need to prune objects that are no longer required.
-         * The time used should be taken from the FrameStamp::getReferenceTime().*/
-        void updateTimeStampOfObjectsInCacheWithExternalReferences(double referenceTime)
-        {
-            // look for objects with external references and update their time stamp.
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (typename ObjectCacheMap::iterator itr = _objectCache.begin(); itr != _objectCache.end(); ++itr)
-            {
-                // If ref count is greater than 1, the object has an external reference.
-                // If the timestamp is yet to be initialized, it needs to be updated too.
-                if ((itr->second.mValue != nullptr && itr->second.mValue->referenceCount() > 1)
-                    || itr->second.mLastUsage == 0.0)
-                    itr->second.mLastUsage = referenceTime;
-            }
-        }
-
-        /** Removed object in the cache which have a time stamp at or before the specified expiry time.
-         * This would typically be called once per frame by applications which are doing database paging,
-         * and need to prune objects that are no longer required, and called after the a called
-         * after the call to updateTimeStampOfObjectsInCacheWithExternalReferences(expirtyTime).*/
-        void removeExpiredObjectsInCache(double expiryTime)
+        // Update last usage timestamp using referenceTime for each cache time if they are not nullptr and referenced
+        // from somewhere else. Remove items with last usage > expiryTime. Note: last usage might be updated from other
+        // places so nullptr or not references elsewhere items are not always removed.
+        void update(double referenceTime, double expiryDelay)
         {
             std::vector<osg::ref_ptr<osg::Object>> objectsToRemove;
             {
+                const double expiryTime = referenceTime - expiryDelay;
                 std::lock_guard<std::mutex> lock(_objectCacheMutex);
-                // Remove expired entries from object cache
-                typename ObjectCacheMap::iterator oitr = _objectCache.begin();
-                while (oitr != _objectCache.end())
-                {
-                    if (oitr->second.mLastUsage <= expiryTime)
-                    {
-                        if (oitr->second.mValue != nullptr)
-                            objectsToRemove.push_back(std::move(oitr->second.mValue));
-                        _objectCache.erase(oitr++);
-                    }
-                    else
-                        ++oitr;
-                }
+                std::erase_if(_objectCache, [&](auto& v) {
+                    Item& item = v.second;
+                    if ((item.mValue != nullptr && item.mValue->referenceCount() > 1) || item.mLastUsage == 0)
+                        item.mLastUsage = referenceTime;
+                    if (item.mLastUsage > expiryTime)
+                        return false;
+                    if (item.mValue != nullptr)
+                        objectsToRemove.push_back(std::move(item.mValue));
+                    return true;
+                });
             }
             // note, actual unref happens outside of the lock
             objectsToRemove.clear();

components/resource/resourcemanager.hpp

@@ -49,11 +49,7 @@ namespace Resource
         virtual ~GenericResourceManager() = default;
 
         /// Clear cache entries that have not been referenced for longer than expiryDelay.
-        void updateCache(double referenceTime) override
-        {
-            mCache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
-            mCache->removeExpiredObjectsInCache(referenceTime - mExpiryDelay);
-        }
+        void updateCache(double referenceTime) override { mCache->update(referenceTime, mExpiryDelay); }
 
         /// Clear all cache entries.
         void clearCache() override { mCache->clear(); }