Ticket #8912: kyra_resource_parent_cache_v6.diff

resource.h

@@ -43 +43 @@
 
 struct ResFileEntry {
         Common::String parent;
+        mutable ResFileEntry *parentEntry; // Cache to avoid lookup by string in the map
+                                                                           // No smart invalidation is needed because the map is cleared globally
+                                                                           // or expanded but no element is ever removed
         uint32 size;
 
         bool preload;
@@ -129 +132 @@
 protected:
         void checkFile(const Common::String &file);
         bool isAccessable(const Common::String &file);
+        bool isAccessable(const ResFileEntry *fileEntry);
 
         void detectFileTypes();
+        void detectFileType(const Common::String &filename, ResFileEntry *fileEntry);
 
         void initializeLoaders();
         const ResArchiveLoader *getLoader(ResFileEntry::kType type) const;
@@ -140 +145 @@
         LoaderList _loaders;
         ResFileMap _map;
 
+        ResFileEntry *getParentEntry(const ResFileEntry *entry) const;
+        ResFileEntry *getParentEntry(const Common::String &filename) const;
+
         typedef Common::List<Common::SharedPtr<CompArchiveLoader> > CompLoaderList;
         typedef CompLoaderList::iterator CompLoaderIterator;
         typedef CompLoaderList::const_iterator CCompLoaderIterator;

resource.cpp

@@ -34 +34 @@
 
 namespace Kyra {
 
+static int sParentCacheHit  = 0;
+static int sParentCacheMiss = 0;
+
 Resource::Resource(KyraEngine_v1 *vm) : _loaders(), _map(), _vm(vm) {
         initializeLoaders();
 }
@@ -201 +204 @@
                                                 // If the old parent is not protected we mark it as not preload anymore,
                                                 // since now no longer all of its embedded files are in the filemap.
                                                 oldParent->_value.preload = false;
-                                                _map[i->filename] = i->entry;
+                                                iter->_value = i->entry;
                                         }
                                 } else {
                                         // Old parent not found? That's strange... But we just overwrite the old
                                         // entry.
-                                        _map[i->filename] = i->entry;
+                                        iter->_value = i->entry;
                                 }
                         } else {
                                 // The old parent has the same filenames as the new archive, we are sure and overwrite the
                                 // old file entry, could be afterall that the preload flag of the new archive was
                                 // just unflagged.
-                                _map[i->filename] = i->entry;
+                                iter->_value = i->entry;
                         }
                 }
                 // 'else' case would mean here overwriting an existing file entry in the map without parent.
@@ -381 +384 @@
                 Common::SeekableReadStream *parent = getFileStream(iter->_value.parent);
                 assert(parent);
 
-                ResFileMap::const_iterator parentIter = _map.find(iter->_value.parent);
-                const ResArchiveLoader *loader = getLoader(parentIter->_value.type);
+                ResFileEntry* parentEntry = getParentEntry(&iter->_value);
+                const ResArchiveLoader *loader = getLoader(parentEntry->type);
                 assert(loader);
 
                 return loader->loadFileFromArchive(file, parent, iter->_value);
@@ -395 +398 @@
         checkFile(file);
 
         ResFileMap::const_iterator iter = _map.find(file);
-        while (iter != _map.end()) {
-                if (!iter->_value.parent.empty()) {
-                        iter = _map.find(iter->_value.parent);
-                        if (iter != _map.end()) {
-                                // parent can never be a non archive file
-                                if (iter->_value.type == ResFileEntry::kRaw)
-                                        return false;
-                                // not mounted parent means not accessable
-                                else if (!iter->_value.mounted)
-                                        return false;
-                        }
-                } else {
-                        return true;
+        if (iter == _map.end())
+                return false;
+        
+        return isAccessable(&iter->_value);
+}
+
+bool Resource::isAccessable(const ResFileEntry *fileEntry) {
+        assert(fileEntry);
+        
+        const ResFileEntry* currentEntry = fileEntry;
+        while (not currentEntry->parent.empty()) {
+                if (currentEntry->parentEntry) {
+                        ++sParentCacheHit;
+                        currentEntry = currentEntry->parentEntry;
                 }
+                else {
+                        ++sParentCacheMiss;
+                        ResFileMap::iterator it = _map.find(currentEntry->parent);
+                        if (it == _map.end())
+                                return false;
+                        else
+                                currentEntry->parentEntry = &it->_value;
+                }
+                // parent can never be a non archive file
+                if (currentEntry->type == ResFileEntry::kRaw)
+                        return false;
+                // not mounted parent means not accessable
+                else if (!currentEntry->mounted)
+                        return false;
         }
-        return false;
+        
+        return true;
 }
 
+ResFileEntry *Resource::getParentEntry(const ResFileEntry *entry) const {
+        assert(entry);
+        if (entry->parent.empty())
+                return NULL;
+        else if (entry->parentEntry) {
+                ++sParentCacheHit;
+                //printf ("%d %d %d\n", sParentCacheHit, sParentCacheMiss, sParentCacheHit+sParentCacheMiss);
+                assert(_map.find(entry->parent) != _map.end());
+                assert(entry->parentEntry == &_map.find(entry->parent)->_value);
+                return entry->parentEntry;
+        }
+        else {
+                ++sParentCacheMiss;
+                ResFileMap::iterator it = _map.find(entry->parent);
+                if (it == _map.end())
+                        return NULL; // If it happens often, the structure maybe deserves a flag to avoid rechecking the map
+                else {
+                        entry->parentEntry = &it->_value;
+                        return entry->parentEntry;
+                }
+        }
+}
+
+ResFileEntry *Resource::getParentEntry(const Common::String &filename) const {
+        ResFileMap::iterator it = _map.find(filename);
+        assert(it != _map.end());
+        return getParentEntry(&it->_value);
+}
+
 void Resource::checkFile(const Common::String &file) {
         if (_map.find(file) == _map.end()) {
                 CompFileMap::const_iterator iter;
 
                 if ((iter = _compFiles.find(file)) != _compFiles.end()) {
-                        ResFileEntry entry;
+                        ResFileEntry& entry = _map[file];
                         entry.parent = "";
+                        entry.parentEntry = NULL;
                         entry.size = iter->_value.size;
                         entry.mounted = false;
                         entry.preload = false;
                         entry.prot = false;
                         entry.type = ResFileEntry::kAutoDetect;
                         entry.offset = 0;
-                        _map[file] = entry;
-
-                        detectFileTypes();
+                        
+                        detectFileType(file, &entry);
                 } else if (Common::File::exists(file)) {
                         Common::File temp;
                         if (temp.open(file)) {
-                                ResFileEntry entry;
+                                ResFileEntry& entry = _map[file];
                                 entry.parent = "";
+                                entry.parentEntry = NULL;
                                 entry.size = temp.size();
                                 entry.mounted = file.compareToIgnoreCase(StaticResource::staticDataFilename()) != 0;
                                 entry.preload = false;
                                 entry.prot = false;
                                 entry.type = ResFileEntry::kAutoDetect;
                                 entry.offset = 0;
-                                _map[file] = entry;
                                 temp.close();
 
-                                detectFileTypes();
+                                detectFileType(file, &entry);
                         }
                 }
         }
 }
 
-void Resource::detectFileTypes() {
-        for (ResFileMap::iterator i = _map.begin(); i != _map.end(); ++i) {
-                if (!isAccessable(i->_key))
-                        continue;
+void Resource::detectFileType(const Common::String &filename, ResFileEntry *fileEntry) {
+        assert(fileEntry);
+        
+        if (!isAccessable(fileEntry))
+                return;
 
-                if (i->_value.type == ResFileEntry::kAutoDetect) {
-                        Common::SeekableReadStream *stream = 0;
-                        for (LoaderIterator l = _loaders.begin(); l != _loaders.end(); ++l) {
-                                if (!(*l)->checkFilename(i->_key))
-                                        continue;
-                                
-                                if (!stream)
-                                        stream = getFileStream(i->_key);
+        if (fileEntry->type == ResFileEntry::kAutoDetect) {
+                Common::SeekableReadStream *stream = 0;
+                for (LoaderIterator l = _loaders.begin(); l != _loaders.end(); ++l) {
+                        if (!(*l)->checkFilename(filename))
+                                continue;
+                        
+                        if (!stream)
+                                stream = getFileStream(filename);
 
-                                if ((*l)->isLoadable(i->_key, *stream)) {
-                                        i->_value.type = (*l)->getType();
-                                        i->_value.mounted = false;
-                                        i->_value.preload = false;
-                                        break;
-                                }
+                        if ((*l)->isLoadable(filename, *stream)) {
+                                fileEntry->type = (*l)->getType();
+                                fileEntry->mounted = false;
+                                fileEntry->preload = false;
+                                break;
                         }
-                        delete stream;
-                        stream = 0;
+                }
+                delete stream;
+                stream = 0;
 
-                        if (i->_value.type == ResFileEntry::kAutoDetect)
-                                i->_value.type = ResFileEntry::kRaw;
-                }
+                if (fileEntry->type == ResFileEntry::kAutoDetect)
+                        fileEntry->type = ResFileEntry::kRaw;
         }
 }
 
+void Resource::detectFileTypes() {
+        for (ResFileMap::iterator i = _map.begin(); i != _map.end(); ++i)
+                detectFileType(i->_key, &i->_value);
+}
+
 void Resource::tryLoadCompFiles() {
         for (CCompLoaderIterator i = _compLoaders.begin(); i != _compLoaders.end(); ++i) {
                 if ((*i)->checkForFiles())
@@ -620 +673 @@
                         entry.size = endoffset - startoffset;
                         entry.offset = startoffset;
                         entry.parent = filename;
+                        entry.parentEntry = NULL;
                         entry.type = ResFileEntry::kAutoDetect;
                         entry.mounted = false;
                         entry.prot = false;
@@ -738 +792 @@
         for (Common::List<Common::String>::iterator file = filenames.begin(); file != filenames.end(); ++file) {
                 ResFileEntry entry;
                 entry.parent = filename;
+                entry.parentEntry = NULL;
                 entry.type = ResFileEntry::kAutoDetect;
                 entry.mounted = false;
                 entry.preload = false;
@@ -807 +862 @@
         for (uint i = 0; i < entries; ++i) {
                 ResFileEntry entry;
                 entry.parent = filename;
+                entry.parentEntry = NULL;
                 entry.type = ResFileEntry::kAutoDetect;
                 entry.mounted = false;
                 entry.preload = false;