From 71e33cf8b2441a6d008f8106e407852417431935 Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Sat, 16 Dec 2023 00:28:47 +0100
Subject: [PATCH 1/7] Add unit tests for GenericObjectCache

---
 apps/openmw_test_suite/CMakeLists.txt         |   2 +
 .../resource/testobjectcache.cpp              | 308 ++++++++++++++++++
 components/resource/objectcache.hpp           |   2 +-
 3 files changed, 311 insertions(+), 1 deletion(-)
 create mode 100644 apps/openmw_test_suite/resource/testobjectcache.cpp

diff --git a/apps/openmw_test_suite/CMakeLists.txt b/apps/openmw_test_suite/CMakeLists.txt
index 4f93319c96..203c4ac2db 100644
--- a/apps/openmw_test_suite/CMakeLists.txt
+++ b/apps/openmw_test_suite/CMakeLists.txt
@@ -94,6 +94,8 @@ file(GLOB UNITTEST_SRC_FILES
     nifosg/testnifloader.cpp
 
     esmterrain/testgridsampling.cpp
+
+    resource/testobjectcache.cpp
 )
 
 source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES})
diff --git a/apps/openmw_test_suite/resource/testobjectcache.cpp b/apps/openmw_test_suite/resource/testobjectcache.cpp
new file mode 100644
index 0000000000..5b7741025a
--- /dev/null
+++ b/apps/openmw_test_suite/resource/testobjectcache.cpp
@@ -0,0 +1,308 @@
+#include <components/resource/objectcache.hpp>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <osg/Object>
+
+namespace Resource
+{
+    namespace
+    {
+        using namespace ::testing;
+
+        TEST(ResourceGenericObjectCacheTest, getRefFromObjectCacheShouldReturnNullptrByDefault)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+            EXPECT_EQ(cache->getRefFromObjectCache(42), nullptr);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, getRefFromObjectCacheOrNoneShouldReturnNulloptByDefault)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(42), std::nullopt);
+        }
+
+        struct Object : osg::Object
+        {
+            Object() = default;
+
+            Object(const Object& other, const osg::CopyOp& copyOp = osg::CopyOp())
+                : osg::Object(other, copyOp)
+            {
+            }
+
+            META_Object(ResourceTest, Object)
+        };
+
+        TEST(ResourceGenericObjectCacheTest, shouldStoreValues)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            EXPECT_EQ(cache->getRefFromObjectCache(key), value);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, shouldStoreNullptrValues)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+            const int key = 42;
+            cache->addEntryToObjectCache(key, nullptr);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(nullptr));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, updateShouldExtendLifetimeForItemsWithZeroTimestamp)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value, 0);
+            value = nullptr;
+
+            const double referenceTime = 1000;
+            const double expiryDelay = 1;
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, addEntryToObjectCacheShouldReplaceExistingItemByKey)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const int key = 42;
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            cache->addEntryToObjectCache(key, value1);
+            ASSERT_EQ(cache->getRefFromObjectCache(key), value1);
+            cache->addEntryToObjectCache(key, value2);
+            EXPECT_EQ(cache->getRefFromObjectCache(key), value2);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, addEntryToObjectCacheShouldMarkLifetime)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 2;
+
+            const int key = 42;
+            cache->addEntryToObjectCache(key, nullptr, referenceTime + expiryDelay);
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
+            cache->removeExpiredObjectsInCache(referenceTime);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + 2 * expiryDelay);
+            cache->removeExpiredObjectsInCache(referenceTime + expiryDelay);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, updateShouldRemoveExpiredItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 1;
+
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            value = nullptr;
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
+            cache->removeExpiredObjectsInCache(referenceTime);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, updateShouldKeepExternallyReferencedItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 1;
+
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
+            cache->removeExpiredObjectsInCache(referenceTime);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(value));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, updateShouldKeepNotExpiredItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 2;
+
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            value = nullptr;
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, updateShouldKeepNotExpiredNullptrItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 2;
+
+            const int key = 42;
+            cache->addEntryToObjectCache(key, nullptr);
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, getRefFromObjectCacheOrNoneShouldNotExtendItemLifetime)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const double referenceTime = 1;
+            const double expiryDelay = 2;
+
+            const int key = 42;
+            cache->addEntryToObjectCache(key, nullptr);
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay / 2);
+            cache->removeExpiredObjectsInCache(referenceTime - expiryDelay / 2);
+            ASSERT_THAT(cache->getRefFromObjectCacheOrNone(key), Optional(_));
+
+            cache->updateTimeStampOfObjectsInCacheWithExternalReferences(referenceTime + expiryDelay);
+            cache->removeExpiredObjectsInCache(referenceTime);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, lowerBoundShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            cache->addEntryToObjectCache("a", nullptr);
+            cache->addEntryToObjectCache("c", nullptr);
+            EXPECT_THAT(cache->lowerBound(std::string_view("b")), Optional(Pair("c", _)));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, shouldSupportRemovingItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+            const int key = 42;
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            ASSERT_EQ(cache->getRefFromObjectCache(key), value);
+            cache->removeFromObjectCache(key);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, clearShouldRemoveAllItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            const int key1 = 42;
+            const int key2 = 13;
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            cache->addEntryToObjectCache(key1, value1);
+            cache->addEntryToObjectCache(key2, value2);
+
+            ASSERT_EQ(cache->getRefFromObjectCache(key1), value1);
+            ASSERT_EQ(cache->getRefFromObjectCache(key2), value2);
+
+            cache->clear();
+
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key1), std::nullopt);
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key2), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, callShouldIterateOverAllItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            osg::ref_ptr<Object> value3(new Object);
+            cache->addEntryToObjectCache(1, value1);
+            cache->addEntryToObjectCache(2, value2);
+            cache->addEntryToObjectCache(3, value3);
+
+            std::vector<std::pair<int, osg::Object*>> actual;
+            cache->call([&](int key, osg::Object* value) { actual.emplace_back(key, value); });
+
+            EXPECT_THAT(actual, ElementsAre(Pair(1, value1.get()), Pair(2, value2.get()), Pair(3, value3.get())));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, getCacheSizeShouldReturnNumberOrAddedItems)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            cache->addEntryToObjectCache(13, value1);
+            cache->addEntryToObjectCache(42, value2);
+
+            EXPECT_EQ(cache->getCacheSize(), 2);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, lowerBoundShouldReturnFirstNotLessThatGivenKey)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            osg::ref_ptr<Object> value3(new Object);
+            cache->addEntryToObjectCache(1, value1);
+            cache->addEntryToObjectCache(2, value2);
+            cache->addEntryToObjectCache(4, value3);
+
+            EXPECT_THAT(cache->lowerBound(3), Optional(Pair(4, value3)));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, lowerBoundShouldReturnNulloptWhenKeyIsGreaterThanAnyOther)
+        {
+            osg::ref_ptr<GenericObjectCache<int>> cache(new GenericObjectCache<int>);
+
+            osg::ref_ptr<Object> value1(new Object);
+            osg::ref_ptr<Object> value2(new Object);
+            osg::ref_ptr<Object> value3(new Object);
+            cache->addEntryToObjectCache(1, value1);
+            cache->addEntryToObjectCache(2, value2);
+            cache->addEntryToObjectCache(3, value3);
+
+            EXPECT_EQ(cache->lowerBound(4), std::nullopt);
+        }
+    }
+}
diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 881729ffc4..f8a5843395 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -180,7 +180,7 @@ namespace Resource
 
         /** call operator()(KeyType, osg::Object*) for each object in the cache. */
         template <class Functor>
-        void call(Functor& f)
+        void call(Functor&& f)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
             for (typename ObjectCacheMap::iterator it = _objectCache.begin(); it != _objectCache.end(); ++it)

From 56401a90a14b467e780283009849ddd1da1552f7 Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Fri, 15 Dec 2023 23:14:44 +0100
Subject: [PATCH 2/7] 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.
---
 .../resource/testobjectcache.cpp              | 46 ++++++----------
 components/resource/objectcache.hpp           | 54 ++++++-------------
 components/resource/resourcemanager.hpp       |  6 +--
 3 files changed, 32 insertions(+), 74 deletions(-)

diff --git a/apps/openmw_test_suite/resource/testobjectcache.cpp b/apps/openmw_test_suite/resource/testobjectcache.cpp
index 5b7741025a..25b6d87e52 100644
--- a/apps/openmw_test_suite/resource/testobjectcache.cpp
+++ b/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);
         }
 
diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index f8a5843395..08ef0dc060 100644
--- a/components/resource/objectcache.hpp
+++ b/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();
diff --git a/components/resource/resourcemanager.hpp b/components/resource/resourcemanager.hpp
index 55e4e142b5..b2427c308a 100644
--- a/components/resource/resourcemanager.hpp
+++ b/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(); }

From fd2fc63dd3c1c7d7788c4e16b979d82c90eafba2 Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Mon, 25 Dec 2023 13:50:58 +0100
Subject: [PATCH 3/7] Support heterogeneous lookup in GenericObjectCache

---
 .../resource/testobjectcache.cpp              | 57 +++++++++++++++++++
 components/resource/objectcache.hpp           | 17 ++++--
 2 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/apps/openmw_test_suite/resource/testobjectcache.cpp b/apps/openmw_test_suite/resource/testobjectcache.cpp
index 25b6d87e52..1c8cff6af0 100644
--- a/apps/openmw_test_suite/resource/testobjectcache.cpp
+++ b/apps/openmw_test_suite/resource/testobjectcache.cpp
@@ -288,5 +288,62 @@ namespace Resource
 
             EXPECT_EQ(cache->lowerBound(4), std::nullopt);
         }
+
+        TEST(ResourceGenericObjectCacheTest, addEntryToObjectCacheShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            const std::string key = "key";
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(std::string_view("key"), value);
+            EXPECT_EQ(cache->getRefFromObjectCache(key), value);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, addEntryToObjectCacheShouldKeyMoving)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            std::string key(128, 'a');
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(std::move(key), value);
+            EXPECT_EQ(key, "");
+            EXPECT_EQ(cache->getRefFromObjectCache(std::string(128, 'a')), value);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, removeFromObjectCacheShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            const std::string key = "key";
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            ASSERT_EQ(cache->getRefFromObjectCache(key), value);
+            cache->removeFromObjectCache(std::string_view("key"));
+            EXPECT_EQ(cache->getRefFromObjectCacheOrNone(key), std::nullopt);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, getRefFromObjectCacheShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            const std::string key = "key";
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            EXPECT_EQ(cache->getRefFromObjectCache(std::string_view("key")), value);
+        }
+
+        TEST(ResourceGenericObjectCacheTest, getRefFromObjectCacheOrNoneShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            const std::string key = "key";
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            EXPECT_THAT(cache->getRefFromObjectCacheOrNone(std::string_view("key")), Optional(value));
+        }
+
+        TEST(ResourceGenericObjectCacheTest, checkInObjectCacheShouldSupportHeterogeneousLookup)
+        {
+            osg::ref_ptr<GenericObjectCache<std::string>> cache(new GenericObjectCache<std::string>);
+            const std::string key = "key";
+            osg::ref_ptr<Object> value(new Object);
+            cache->addEntryToObjectCache(key, value);
+            EXPECT_TRUE(cache->checkInObjectCache(std::string_view("key"), 0));
+        }
     }
 }
diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 08ef0dc060..066f5b1d3e 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -81,14 +81,19 @@ namespace Resource
         }
 
         /** Add a key,object,timestamp triple to the Registry::ObjectCache.*/
-        void addEntryToObjectCache(const KeyType& key, osg::Object* object, double timestamp = 0.0)
+        template <class K>
+        void addEntryToObjectCache(K&& key, osg::Object* object, double timestamp = 0.0)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            _objectCache[key] = Item{ object, timestamp };
+            const auto it = _objectCache.find(key);
+            if (it == _objectCache.end())
+                _objectCache.emplace_hint(it, std::forward<K>(key), Item{ object, timestamp });
+            else
+                it->second = Item{ object, timestamp };
         }
 
         /** Remove Object from cache.*/
-        void removeFromObjectCache(const KeyType& key)
+        void removeFromObjectCache(const auto& key)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
             typename ObjectCacheMap::iterator itr = _objectCache.find(key);
@@ -97,7 +102,7 @@ namespace Resource
         }
 
         /** Get an ref_ptr<Object> from the object cache*/
-        osg::ref_ptr<osg::Object> getRefFromObjectCache(const KeyType& key)
+        osg::ref_ptr<osg::Object> getRefFromObjectCache(const auto& key)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
             typename ObjectCacheMap::iterator itr = _objectCache.find(key);
@@ -107,7 +112,7 @@ namespace Resource
                 return nullptr;
         }
 
-        std::optional<osg::ref_ptr<osg::Object>> getRefFromObjectCacheOrNone(const KeyType& key)
+        std::optional<osg::ref_ptr<osg::Object>> getRefFromObjectCacheOrNone(const auto& key)
         {
             const std::lock_guard<std::mutex> lock(_objectCacheMutex);
             const auto it = _objectCache.find(key);
@@ -117,7 +122,7 @@ namespace Resource
         }
 
         /** Check if an object is in the cache, and if it is, update its usage time stamp. */
-        bool checkInObjectCache(const KeyType& key, double timeStamp)
+        bool checkInObjectCache(const auto& key, double timeStamp)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
             typename ObjectCacheMap::iterator itr = _objectCache.find(key);

From 2f0613c8d453f72a22f71076384e45c07a9b235b Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Mon, 25 Dec 2023 14:18:29 +0100
Subject: [PATCH 4/7] Remove user defined destructor for GenericObjectCache

---
 components/resource/objectcache.hpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 066f5b1d3e..9cb74c762c 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -194,8 +194,6 @@ namespace Resource
             double mLastUsage;
         };
 
-        virtual ~GenericObjectCache() {}
-
         using ObjectCacheMap = std::map<KeyType, Item, std::less<>>;
 
         ObjectCacheMap _objectCache;

From 7b1ee2780b16652406cbfae21e5936f425489aa5 Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Mon, 25 Dec 2023 14:17:12 +0100
Subject: [PATCH 5/7] Use ranged for loops in GenericObjectCache

---
 components/resource/objectcache.hpp | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 9cb74c762c..8f99e6dfda 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -139,26 +139,18 @@ namespace Resource
         void releaseGLObjects(osg::State* state)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (typename ObjectCacheMap::iterator itr = _objectCache.begin(); itr != _objectCache.end(); ++itr)
-            {
-                osg::Object* object = itr->second.mValue.get();
-                object->releaseGLObjects(state);
-            }
+            for (const auto& [k, v] : _objectCache)
+                v.mValue->releaseGLObjects(state);
         }
 
         /** call node->accept(nv); for all nodes in the objectCache. */
         void accept(osg::NodeVisitor& nv)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (typename ObjectCacheMap::iterator itr = _objectCache.begin(); itr != _objectCache.end(); ++itr)
-            {
-                if (osg::Object* object = itr->second.mValue.get())
-                {
-                    osg::Node* node = dynamic_cast<osg::Node*>(object);
-                    if (node)
+            for (const auto& [k, v] : _objectCache)
+                if (osg::Object* const object = v.mValue.get())
+                    if (osg::Node* const node = dynamic_cast<osg::Node*>(object))
                         node->accept(nv);
-                }
-            }
         }
 
         /** call operator()(KeyType, osg::Object*) for each object in the cache. */
@@ -166,8 +158,8 @@ namespace Resource
         void call(Functor&& f)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (typename ObjectCacheMap::iterator it = _objectCache.begin(); it != _objectCache.end(); ++it)
-                f(it->first, it->second.mValue.get());
+            for (const auto& [k, v] : _objectCache)
+                f(k, v.mValue.get());
         }
 
         /** Get the number of objects in the cache. */

From 45b1b4f1e0344caf3c8dd503ea3d9e167000badd Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Mon, 25 Dec 2023 14:19:13 +0100
Subject: [PATCH 6/7] Remove redundant ObjectCacheMap alias

---
 components/resource/objectcache.hpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 8f99e6dfda..5374cef460 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -96,7 +96,7 @@ namespace Resource
         void removeFromObjectCache(const auto& key)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            typename ObjectCacheMap::iterator itr = _objectCache.find(key);
+            const auto itr = _objectCache.find(key);
             if (itr != _objectCache.end())
                 _objectCache.erase(itr);
         }
@@ -105,7 +105,7 @@ namespace Resource
         osg::ref_ptr<osg::Object> getRefFromObjectCache(const auto& key)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            typename ObjectCacheMap::iterator itr = _objectCache.find(key);
+            const auto itr = _objectCache.find(key);
             if (itr != _objectCache.end())
                 return itr->second.mValue;
             else
@@ -125,7 +125,7 @@ namespace Resource
         bool checkInObjectCache(const auto& key, double timeStamp)
         {
             std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            typename ObjectCacheMap::iterator itr = _objectCache.find(key);
+            const auto itr = _objectCache.find(key);
             if (itr != _objectCache.end())
             {
                 itr->second.mLastUsage = timeStamp;
@@ -186,9 +186,7 @@ namespace Resource
             double mLastUsage;
         };
 
-        using ObjectCacheMap = std::map<KeyType, Item, std::less<>>;
-
-        ObjectCacheMap _objectCache;
+        std::map<KeyType, Item, std::less<>> _objectCache;
         mutable std::mutex _objectCacheMutex;
     };
 

From 7a817d31470d1804e7944e6d1f116c59662dfcb9 Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Mon, 25 Dec 2023 14:23:36 +0100
Subject: [PATCH 7/7] Apply project naming styleguide to GenericObjectCache

---
 components/resource/objectcache.hpp | 68 ++++++++++++++---------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/components/resource/objectcache.hpp b/components/resource/objectcache.hpp
index 5374cef460..dffa0e9fdb 100644
--- a/components/resource/objectcache.hpp
+++ b/components/resource/objectcache.hpp
@@ -57,8 +57,8 @@ namespace Resource
             std::vector<osg::ref_ptr<osg::Object>> objectsToRemove;
             {
                 const double expiryTime = referenceTime - expiryDelay;
-                std::lock_guard<std::mutex> lock(_objectCacheMutex);
-                std::erase_if(_objectCache, [&](auto& v) {
+                std::lock_guard<std::mutex> lock(mMutex);
+                std::erase_if(mItems, [&](auto& v) {
                     Item& item = v.second;
                     if ((item.mValue != nullptr && item.mValue->referenceCount() > 1) || item.mLastUsage == 0)
                         item.mLastUsage = referenceTime;
@@ -76,18 +76,18 @@ namespace Resource
         /** Remove all objects in the cache regardless of having external references or expiry times.*/
         void clear()
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            _objectCache.clear();
+            std::lock_guard<std::mutex> lock(mMutex);
+            mItems.clear();
         }
 
         /** Add a key,object,timestamp triple to the Registry::ObjectCache.*/
         template <class K>
         void addEntryToObjectCache(K&& key, osg::Object* object, double timestamp = 0.0)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto it = _objectCache.find(key);
-            if (it == _objectCache.end())
-                _objectCache.emplace_hint(it, std::forward<K>(key), Item{ object, timestamp });
+            std::lock_guard<std::mutex> lock(mMutex);
+            const auto it = mItems.find(key);
+            if (it == mItems.end())
+                mItems.emplace_hint(it, std::forward<K>(key), Item{ object, timestamp });
             else
                 it->second = Item{ object, timestamp };
         }
@@ -95,18 +95,18 @@ namespace Resource
         /** Remove Object from cache.*/
         void removeFromObjectCache(const auto& key)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto itr = _objectCache.find(key);
-            if (itr != _objectCache.end())
-                _objectCache.erase(itr);
+            std::lock_guard<std::mutex> lock(mMutex);
+            const auto itr = mItems.find(key);
+            if (itr != mItems.end())
+                mItems.erase(itr);
         }
 
         /** Get an ref_ptr<Object> from the object cache*/
         osg::ref_ptr<osg::Object> getRefFromObjectCache(const auto& key)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto itr = _objectCache.find(key);
-            if (itr != _objectCache.end())
+            std::lock_guard<std::mutex> lock(mMutex);
+            const auto itr = mItems.find(key);
+            if (itr != mItems.end())
                 return itr->second.mValue;
             else
                 return nullptr;
@@ -114,9 +114,9 @@ namespace Resource
 
         std::optional<osg::ref_ptr<osg::Object>> getRefFromObjectCacheOrNone(const auto& key)
         {
-            const std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto it = _objectCache.find(key);
-            if (it == _objectCache.end())
+            const std::lock_guard<std::mutex> lock(mMutex);
+            const auto it = mItems.find(key);
+            if (it == mItems.end())
                 return std::nullopt;
             return it->second.mValue;
         }
@@ -124,9 +124,9 @@ namespace Resource
         /** Check if an object is in the cache, and if it is, update its usage time stamp. */
         bool checkInObjectCache(const auto& key, double timeStamp)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto itr = _objectCache.find(key);
-            if (itr != _objectCache.end())
+            std::lock_guard<std::mutex> lock(mMutex);
+            const auto itr = mItems.find(key);
+            if (itr != mItems.end())
             {
                 itr->second.mLastUsage = timeStamp;
                 return true;
@@ -138,16 +138,16 @@ namespace Resource
         /** call releaseGLObjects on all objects attached to the object cache.*/
         void releaseGLObjects(osg::State* state)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (const auto& [k, v] : _objectCache)
+            std::lock_guard<std::mutex> lock(mMutex);
+            for (const auto& [k, v] : mItems)
                 v.mValue->releaseGLObjects(state);
         }
 
         /** call node->accept(nv); for all nodes in the objectCache. */
         void accept(osg::NodeVisitor& nv)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (const auto& [k, v] : _objectCache)
+            std::lock_guard<std::mutex> lock(mMutex);
+            for (const auto& [k, v] : mItems)
                 if (osg::Object* const object = v.mValue.get())
                     if (osg::Node* const node = dynamic_cast<osg::Node*>(object))
                         node->accept(nv);
@@ -157,24 +157,24 @@ namespace Resource
         template <class Functor>
         void call(Functor&& f)
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            for (const auto& [k, v] : _objectCache)
+            std::lock_guard<std::mutex> lock(mMutex);
+            for (const auto& [k, v] : mItems)
                 f(k, v.mValue.get());
         }
 
         /** Get the number of objects in the cache. */
         unsigned int getCacheSize() const
         {
-            std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            return _objectCache.size();
+            std::lock_guard<std::mutex> lock(mMutex);
+            return mItems.size();
         }
 
         template <class K>
         std::optional<std::pair<KeyType, osg::ref_ptr<osg::Object>>> lowerBound(K&& key)
         {
-            const std::lock_guard<std::mutex> lock(_objectCacheMutex);
-            const auto it = _objectCache.lower_bound(std::forward<K>(key));
-            if (it == _objectCache.end())
+            const std::lock_guard<std::mutex> lock(mMutex);
+            const auto it = mItems.lower_bound(std::forward<K>(key));
+            if (it == mItems.end())
                 return std::nullopt;
             return std::pair(it->first, it->second.mValue);
         }
@@ -186,8 +186,8 @@ namespace Resource
             double mLastUsage;
         };
 
-        std::map<KeyType, Item, std::less<>> _objectCache;
-        mutable std::mutex _objectCacheMutex;
+        std::map<KeyType, Item, std::less<>> mItems;
+        mutable std::mutex mMutex;
     };
 
     class ObjectCache : public GenericObjectCache<std::string>