Refactor NIF cache

- Remove broken cache locking mechanism

This was supposed to unload NIFFiles after a cell transition completes, but it was never working due to a mistake on the line if (--sLockLevel), should have been if (--sLockLevel == 0). Repairing this would increase load times (NIF files would have to be reloaded more frequently), so just removed it for now.

 - Decouple cache from NIFFile (now a new nifcache component)

 - Add API for future background loading

 - Provide a reliable way (SharedPtr) to hold on to loaded NIFFiles. This will be useful to avoid deep copies of keyframe and text key data, which is currently a performance bottleneck.

apps/opencs/editor.hpp

@@ -16,6 +16,8 @@
 
 #include <components/files/multidircollection.hpp>
 
+#include <components/nifcache/nifcache.hpp>
+
 #include "model/settings/usersettings.hpp"
 #include "model/doc/documentmanager.hpp"
 
@@ -37,6 +39,7 @@ namespace CS
     {
             Q_OBJECT
 
+            Nif::Cache mNifCache;
             Files::ConfigurationManager mCfgMgr;
             CSMSettings::UserSettings mUserSettings;
             CSMDoc::DocumentManager mDocumentManager;

apps/openmw/engine.cpp

@@ -15,7 +15,6 @@
 #include <components/bsa/resources.hpp>
 #include <components/files/configurationmanager.hpp>
 #include <components/translation/translation.hpp>
-#include <components/nif/niffile.hpp>
 #include <components/nifoverrides/nifoverrides.hpp>
 
 #include <components/nifbullet/bulletnifloader.hpp>
@@ -315,8 +314,6 @@ void OMW::Engine::prepareEngine (Settings::Manager & settings)
     mEnvironment.setStateManager (
         new MWState::StateManager (mCfgMgr.getUserDataPath() / "saves", mContentFiles.at (0)));
 
-    Nif::NIFFile::CacheLock cachelock;
-
     std::string renderSystem = settings.getString("render system", "Video");
     if (renderSystem == "")
     {

apps/openmw/engine.hpp

@@ -7,6 +7,8 @@
 #include <components/files/collections.hpp>
 #include <components/translation/translation.hpp>
 #include <components/settings/settings.hpp>
+#include <components/nifcache/nifcache.hpp>
+
 
 #include "mwbase/environment.hpp"
 
@@ -94,6 +96,8 @@ namespace OMW
             std::vector<std::string> mScriptBlacklist;
             bool mScriptBlacklistUse;
 
+            Nif::Cache mNifCache;
+
             // not implemented
             Engine (const Engine&);
             Engine& operator= (const Engine&);

apps/openmw/mwworld/scene.cpp

@@ -259,8 +259,6 @@ namespace MWWorld
 
     void Scene::changeCell (int X, int Y, const ESM::Position& position, bool adjustPlayerPos)
     {
-        Nif::NIFFile::CacheLock cachelock;
-
         Loading::Listener* loadingListener = MWBase::Environment::get().getWindowManager()->getLoadingScreen();
         Loading::ScopedLoad load(loadingListener);
 
@@ -408,7 +406,6 @@ namespace MWWorld
         if(!loadcell)
             loadcell = *mCurrentCell != *cell;
 
-        Nif::NIFFile::CacheLock lock;
         MWBase::Environment::get().getWindowManager()->fadeScreenOut(0.5);
 
         Loading::Listener* loadingListener = MWBase::Environment::get().getWindowManager()->getLoadingScreen();

components/CMakeLists.txt

@@ -22,6 +22,10 @@ add_component_dir (nif
     controlled effect niftypes record controller extra node record_ptr data niffile property
     )
 
+add_component_dir (nifcache
+    nifcache
+    )
+
 add_component_dir (nifogre
     ogrenifloader skeleton material mesh particles controller
     )

components/nif/niffile.cpp

@@ -35,143 +35,11 @@
 
 #include <iostream>
 
-//TODO: when threading is needed, enable these
-//#include <boost/mutex.hpp>
-#include <boost/thread/locks.hpp>
-
 namespace Nif
 {
 
-class NIFFile::LoadedCache
-{
-    //TODO: enable this to make cache thread safe...
-    //typedef boost::mutex mutex;
-    
-    struct mutex
-    {
-        void lock () {};
-        void unlock () {}
-    };
-    
-    typedef boost::lock_guard <mutex> lock_guard;
-    typedef std::map < std::string, boost::weak_ptr <NIFFile> > loaded_map;
-    typedef std::vector < boost::shared_ptr <NIFFile> > locked_files;
-
-    static int sLockLevel;
-    static mutex sProtector;
-    static loaded_map sLoadedMap;
-    static locked_files sLockedFiles;
-
-public:
-
-    static ptr create (const std::string &name)
-    {
-        lock_guard _ (sProtector);
-
-        ptr result;
-
-        // lookup the resource
-        loaded_map::iterator i = sLoadedMap.find (name);
-
-        if (i == sLoadedMap.end ()) // it doesn't existing currently,
-        {                           // or hasn't in the very near past
-
-            // create it now, for smoother threading if needed, the
-            // loading should be performed outside of the sLoaderMap
-            // lock and an alternate mechanism should be used to
-            // synchronize threads competing to load the same resource
-            result = boost::make_shared <NIFFile> (name, psudo_private_modifier());
-
-            // if we are locking the cache add an extra reference
-            // to keep the file in memory
-            if (sLockLevel > 0)
-                sLockedFiles.push_back (result);
-
-            // stash a reference to the resource so that future
-            // calls can benefit
-            sLoadedMap [name] = boost::weak_ptr <NIFFile> (result);
-        }
-        else // it may (probably) still exists
-        {
-            // attempt to get the reference
-            result = i->second.lock ();
-
-            if (!result) // resource is in the process of being destroyed
-            {
-                // create a new instance, to replace the one that has
-                // begun the irreversible process of being destroyed
-                result = boost::make_shared <NIFFile> (name, psudo_private_modifier());
-
-                // respect the cache lock...
-                if (sLockLevel > 0)
-                    sLockedFiles.push_back (result);
-
-                // we potentially overwrite an expired pointer here
-                // but the other thread performing the delete on
-                // the previous copy of this resource will detect it
-                // and make sure not to erase the new reference
-                sLoadedMap [name] = boost::weak_ptr <NIFFile> (result);
-            }
-        }
-
-        // we made it!
-        return result;
-    }
-
-    static void release (NIFFile * file)
-    {
-        lock_guard _ (sProtector);
-
-        loaded_map::iterator i = sLoadedMap.find (file->filename);
-
-        // its got to be in here, it just might not be us...
-        assert (i != sLoadedMap.end ());
-
-        // if weak_ptr is still expired, this resource hasn't been recreated
-        // between the initiation of the final release due to destruction
-        // of the last shared pointer and this thread acquiring the lock on
-        // the loader map
-        if (i->second.expired ())
-            sLoadedMap.erase (i);
-    }
-
-    static void lockCache ()
-    {
-        lock_guard _ (sProtector);
-
-        sLockLevel++;
-    }
-
-    static void unlockCache ()
-    {
-        locked_files resetList;
-
-        {
-            lock_guard _ (sProtector);
-
-            if (--sLockLevel)
-                sLockedFiles.swap(resetList);
-        }
-
-        // this not necessary, but makes it clear that the
-        // deletion of the locked cache entries is being done
-        // outside the protection of sProtector
-        resetList.clear ();
-    }
-};
-
-int NIFFile::LoadedCache::sLockLevel = 0;
-NIFFile::LoadedCache::mutex NIFFile::LoadedCache::sProtector;
-NIFFile::LoadedCache::loaded_map NIFFile::LoadedCache::sLoadedMap;
-NIFFile::LoadedCache::locked_files NIFFile::LoadedCache::sLockedFiles;
-
-// these three calls are forwarded to the cache implementation...
-void NIFFile::lockCache ()     { LoadedCache::lockCache (); }
-void NIFFile::unlockCache ()   { LoadedCache::unlockCache (); }
-NIFFile::ptr NIFFile::create (const std::string &name) { return LoadedCache::create  (name); }
-
 /// Open a NIF stream. The name is used for error messages.
-NIFFile::NIFFile(const std::string &name, psudo_private_modifier)
+NIFFile::NIFFile(const std::string &name)
     : ver(0)
     , filename(name)
 {
@@ -180,8 +48,6 @@ NIFFile::NIFFile(const std::string &name, psudo_private_modifier)
 
 NIFFile::~NIFFile()
 {
-    LoadedCache::release (this);
-
     for(std::size_t i=0; i<records.size(); i++)
         delete records[i];
 }

components/nif/niffile.hpp

@@ -73,11 +73,6 @@ class NIFFile
     /// Parse the file
     void parse();
 
-    class LoadedCache;
-    friend class LoadedCache;
-
-    // attempt to protect NIFFile from misuse...
-    struct psudo_private_modifier {}; // this dirty little trick should optimize out
     NIFFile (NIFFile const &);
     void operator = (NIFFile const &);
 
@@ -96,22 +91,10 @@ public:
                   << "File: " << filename <<std::endl;
     }
 
-    typedef boost::shared_ptr <NIFFile> ptr;
-
     /// Open a NIF stream. The name is used for error messages.
-    NIFFile(const std::string &name, psudo_private_modifier);
+    NIFFile(const std::string &name);
     ~NIFFile();
 
-    static ptr create (const std::string &name);
-    static void lockCache ();
-    static void unlockCache ();
-
-    struct CacheLock
-    {
-        CacheLock () { lockCache (); }
-        ~CacheLock () { unlockCache (); }
-    };
-
     /// Get a given record
     Record *getRecord(size_t index) const
     {

components/nifbullet/bulletnifloader.cpp

@@ -28,6 +28,8 @@ http://www.gnu.org/licenses/ .
 
 #include <components/misc/stringops.hpp>
 
+#include <components/nifcache/nifcache.hpp>
+
 #include "../nif/niffile.hpp"
 #include "../nif/node.hpp"
 #include "../nif/data.hpp"
@@ -74,7 +76,7 @@ void ManualBulletShapeLoader::loadResource(Ogre::Resource *resource)
     // of the early stages of development. Right now we WANT to catch
     // every error as early and intrusively as possible, as it's most
     // likely a sign of incomplete code rather than faulty input.
-    Nif::NIFFile::ptr pnif (Nif::NIFFile::create (mResourceName.substr(0, mResourceName.length()-7)));
+    Nif::NIFFilePtr pnif (Nif::Cache::getInstance().load(mResourceName.substr(0, mResourceName.length()-7)));
     Nif::NIFFile & nif = *pnif.get ();
     if (nif.numRoots() < 1)
     {
@@ -388,7 +390,7 @@ bool findBoundingBox (const Nif::Node* node, Ogre::Vector3& halfExtents, Ogre::V
 
 bool getBoundingBox(const std::string& nifFile, Ogre::Vector3& halfExtents, Ogre::Vector3& translation, Ogre::Quaternion& orientation)
 {
-    Nif::NIFFile::ptr pnif (Nif::NIFFile::create (nifFile));
+    Nif::NIFFilePtr pnif (Nif::Cache::getInstance().load(nifFile));
     Nif::NIFFile & nif = *pnif.get ();
 
     if (nif.numRoots() < 1)

components/nifcache/nifcache.cpp

@@ -0,0 +1,40 @@
+#include "nifcache.hpp"
+
+namespace Nif
+{
+
+Cache* Cache::sThis = 0;
+
+Cache& Cache::getInstance()
+{
+    assert (sThis);
+    return *sThis;
+}
+
+Cache* Cache::getInstancePtr()
+{
+    return sThis;
+}
+
+Cache::Cache()
+{
+    assert (!sThis);
+    sThis = this;
+}
+
+NIFFilePtr Cache::load(const std::string &filename)
+{
+    // TODO: normalize file path to make sure we're not loading the same file twice
+
+    LoadedMap::iterator it = mLoadedMap.find(filename);
+    if (it != mLoadedMap.end())
+        return it->second;
+    else
+    {
+        NIFFilePtr file(new Nif::NIFFile(filename));
+        mLoadedMap[filename] = file;
+        return file;
+    }
+}
+
+}

components/nifcache/nifcache.hpp

@@ -0,0 +1,48 @@
+#ifndef OPENMW_COMPONENTS_NIFCACHE_H
+#define OPENMW_COMPONENTS_NIFCACHE_H
+
+#include <components/nif/niffile.hpp>
+
+#include <boost/shared_ptr.hpp>
+
+namespace Nif
+{
+
+    typedef boost::shared_ptr<Nif::NIFFile> NIFFilePtr;
+
+    /// @brief A basic resource manager for NIF files
+    class Cache
+    {
+    public:
+        Cache();
+
+        /// Queue this file for background loading. A worker thread will start loading the file.
+        /// To get the loaded NIFFilePtr, use the load method, which will wait until the worker thread is finished
+        /// and then return the loaded file.
+        //void loadInBackground (const std::string& file);
+
+        /// Read and parse the given file. May retrieve from cache if this file has been used previously.
+        /// @note If the file is currently loading in the background, this function will block until
+        ///       the background loading finishes, then return the background loaded file.
+        /// @note Returns a SharedPtr to the file and the file will stay loaded as long as the user holds on to this pointer.
+        ///       When all external SharedPtrs to a file are released, the cache may decide to unload the file.
+        NIFFilePtr load (const std::string& filename);
+
+        /// Return instance of this class.
+        static Cache& getInstance();
+        static Cache* getInstancePtr();
+
+    private:
+        static Cache* sThis;
+
+        Cache(const Cache&);
+        Cache& operator =(const Cache&);
+
+        typedef std::map<std::string, NIFFilePtr> LoadedMap;
+
+        LoadedMap mLoadedMap;
+    };
+
+}
+
+#endif

components/nifogre/mesh.cpp

@@ -15,6 +15,7 @@
 #include <OgreKeyFrame.h>
 
 #include <components/nif/node.hpp>
+#include <components/nifcache/nifcache.hpp>
 #include <components/misc/stringops.hpp>
 
 #include "material.hpp"
@@ -383,7 +384,7 @@ void NIFMeshLoader::loadResource(Ogre::Resource *resource)
     Ogre::Mesh *mesh = dynamic_cast<Ogre::Mesh*>(resource);
     OgreAssert(mesh, "Attempting to load a mesh into a non-mesh resource!");
 
-    Nif::NIFFile::ptr nif = Nif::NIFFile::create(mName);
+    Nif::NIFFilePtr nif = Nif::Cache::getInstance().load(mName);
     if(mShapeIndex >= nif->numRecords())
     {
         Ogre::SkeletonManager *skelMgr = Ogre::SkeletonManager::getSingletonPtr();

components/nifogre/ogrenifloader.cpp

@@ -45,6 +45,7 @@
 #include <extern/shiny/Main/Factory.hpp>
 
 #include <components/nif/node.hpp>
+#include <components/nifcache/nifcache.hpp>
 #include <components/misc/stringops.hpp>
 #include <components/misc/resourcehelpers.hpp>
 
@@ -1123,7 +1124,7 @@ class NIFObjectLoader
 public:
     static void load(Ogre::SceneNode *sceneNode, ObjectScenePtr scene, const std::string &name, const std::string &group, int flags=0)
     {
-        Nif::NIFFile::ptr nif = Nif::NIFFile::create(name);
+        Nif::NIFFilePtr nif = Nif::Cache::getInstance().load(name);
         if(nif->numRoots() < 1)
         {
             nif->warn("Found no root nodes in "+name+".");
@@ -1153,7 +1154,7 @@ public:
     static void loadKf(Ogre::Skeleton *skel, const std::string &name,
                        TextKeyMap &textKeys, std::vector<Ogre::Controller<Ogre::Real> > &ctrls)
     {
-        Nif::NIFFile::ptr nif = Nif::NIFFile::create(name);
+        Nif::NIFFilePtr nif = Nif::Cache::getInstance().load(name);
         if(nif->numRoots() < 1)
         {
             nif->warn("Found no root nodes in "+name+".");

components/nifogre/skeleton.cpp

@@ -6,6 +6,7 @@
 #include <OgreBone.h>
 
 #include <components/nif/node.hpp>
+#include <components/nifcache/nifcache.hpp>
 #include <components/misc/stringops.hpp>
 
 namespace NifOgre
@@ -83,7 +84,7 @@ void NIFSkeletonLoader::loadResource(Ogre::Resource *resource)
     Ogre::Skeleton *skel = dynamic_cast<Ogre::Skeleton*>(resource);
     OgreAssert(skel, "Attempting to load a skeleton into a non-skeleton resource!");
 
-    Nif::NIFFile::ptr nif(Nif::NIFFile::create(skel->getName()));
+    Nif::NIFFilePtr nif(Nif::Cache::getInstance().load(skel->getName()));
     const Nif::Node *node = static_cast<const Nif::Node*>(nif->getRoot(0));
 
     try {