From cd63c177e20b4205c51c00637bc40f82f44225a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Wed, 8 Aug 2012 08:25:27 +0000 Subject: [PATCH] * std::unordered_map implementation in MSVC 10 has terribly slow destructor. Since bug is fixed only in VC11, I replaced it with boost::unordered_map. * fixed crash with invalid .lod archive (some H3 installations have fake 1 byte .lod "archives" * fixed crash when parsing invalid map file * minor random optimizations (rv-refs, etc) --- Global.h | 16 +++-- client/CPreGame.cpp | 19 +++--- lib/Filesystem/CFileInfo.cpp | 4 +- lib/Filesystem/CFileInfo.h | 2 +- lib/Filesystem/CFilesystemLoader.cpp | 7 +-- lib/Filesystem/CFilesystemLoader.h | 6 +- lib/Filesystem/CLodArchiveLoader.cpp | 11 +++- lib/Filesystem/CLodArchiveLoader.h | 4 +- lib/Filesystem/CResourceLoader.cpp | 24 +++++--- lib/Filesystem/CResourceLoader.h | 84 ++++++++++++++++++-------- lib/Filesystem/ISimpleResourceLoader.h | 2 +- 11 files changed, 114 insertions(+), 65 deletions(-) diff --git a/Global.h b/Global.h index a34a282c1..ba17bdc77 100644 --- a/Global.h +++ b/Global.h @@ -29,15 +29,21 @@ #include "tchar_amigaos4.h" #endif -#include +#include +#include #include -#include -#include -#include +#include +#include +#include +#include +#include +#include #include -#include +#include +#include #include #include +#include #include #include diff --git a/client/CPreGame.cpp b/client/CPreGame.cpp index 1a6894189..8e32236b2 100644 --- a/client/CPreGame.cpp +++ b/client/CPreGame.cpp @@ -1049,17 +1049,20 @@ void SelectionTab::parseMaps(const std::vector &files, int start, in while(start < allItems.size()) { - CCompressedStream stream(std::move(CResourceHandler::get()->load(files[start])), true); - int read = stream.read(mapBuffer, 1500); + try + { + CCompressedStream stream(std::move(CResourceHandler::get()->load(files[start])), true); + int read = stream.read(mapBuffer, 1500); + + if(read < 50 || !mapBuffer[4]) + throw std::runtime_error("corrupted map file"); - if(read < 50 || !mapBuffer[4]) - { - tlog3 << "\t\tWarning: corrupted map file: " << files[start].getName() << std::endl; - } - else //valid map - { allItems[start].mapInit(files[start].getName(), mapBuffer); } + catch(std::exception &e) + { + tlog3 << "\t\tWarning: failed to load map " << files[start].getName() << ": " << e.what() << std::endl; + } start += threads; } } diff --git a/lib/Filesystem/CFileInfo.cpp b/lib/Filesystem/CFileInfo.cpp index e11fc413e..59aef320d 100644 --- a/lib/Filesystem/CFileInfo.cpp +++ b/lib/Filesystem/CFileInfo.cpp @@ -6,8 +6,8 @@ CFileInfo::CFileInfo() : name("") } -CFileInfo::CFileInfo(const std::string & name) - : name(name) +CFileInfo::CFileInfo(std::string name) + : name(std::move(name)) { } diff --git a/lib/Filesystem/CFileInfo.h b/lib/Filesystem/CFileInfo.h index 14128450e..975853674 100644 --- a/lib/Filesystem/CFileInfo.h +++ b/lib/Filesystem/CFileInfo.h @@ -29,7 +29,7 @@ public: * * @param name The path and name of the file. */ - explicit CFileInfo(const std::string & name); + explicit CFileInfo(std::string name); /** * Checks if the file exists. diff --git a/lib/Filesystem/CFilesystemLoader.cpp b/lib/Filesystem/CFilesystemLoader.cpp index 11b51f012..ad741183a 100644 --- a/lib/Filesystem/CFilesystemLoader.cpp +++ b/lib/Filesystem/CFilesystemLoader.cpp @@ -29,7 +29,7 @@ bool CFilesystemLoader::existsEntry(const std::string & resourceName) const return false; } -std::unordered_map CFilesystemLoader::getEntries() const +boost::unordered_map CFilesystemLoader::getEntries() const { return fileList; } @@ -54,11 +54,10 @@ bool CFilesystemLoader::createEntry(std::string filename) } -std::unordered_map CFilesystemLoader::listFiles(size_t depth, bool initial) const +boost::unordered_map CFilesystemLoader::listFiles(size_t depth, bool initial) const { - assert(boost::filesystem::is_directory(baseDirectory)); - std::unordered_map fileList; + boost::unordered_map fileList; std::vector path;//vector holding relative path to our file diff --git a/lib/Filesystem/CFilesystemLoader.h b/lib/Filesystem/CFilesystemLoader.h index 42bfafa3a..7a54bb41f 100644 --- a/lib/Filesystem/CFilesystemLoader.h +++ b/lib/Filesystem/CFilesystemLoader.h @@ -53,7 +53,7 @@ public: * * @return a list of all entries in the filesystem. */ - std::unordered_map getEntries() const; + boost::unordered_map getEntries() const; /** * Gets the origin of the archive loader. @@ -72,7 +72,7 @@ private: * key = ResourceID for resource loader * value = name that can be used to access file */ - std::unordered_map fileList; + boost::unordered_map fileList; /** * Returns a list of pathnames denoting the files in the directory denoted by this pathname. @@ -83,5 +83,5 @@ private: * @return a list of pathnames denoting the files and directories in the directory denoted by this pathname * The array will be empty if the directory is empty. Ptr is null if the directory doesn't exist or if it isn't a directory. */ - std::unordered_map listFiles(size_t depth, bool initial) const; + boost::unordered_map listFiles(size_t depth, bool initial) const; }; diff --git a/lib/Filesystem/CLodArchiveLoader.cpp b/lib/Filesystem/CLodArchiveLoader.cpp index 3e3374236..e45f83e79 100644 --- a/lib/Filesystem/CLodArchiveLoader.cpp +++ b/lib/Filesystem/CLodArchiveLoader.cpp @@ -19,6 +19,10 @@ CLodArchiveLoader::CLodArchiveLoader(const std::string & archive) this->archive = archive; CFileInputStream fileStream(archive); + // Fake .lod file with no data has to be silently ignored. + if(fileStream.getSize() < 10) + return; + // Retrieve file extension of archive in uppercase CFileInfo fileInfo(archive); std::string ext = fileInfo.getExtension(); @@ -57,6 +61,7 @@ void CLodArchiveLoader::initLODArchive(CFileInputStream & fileStream) // Read count of total files CBinaryReader reader(fileStream); + fileStream.seek(8); ui32 totalFiles = reader.readUInt32(); @@ -184,9 +189,9 @@ std::unique_ptr CLodArchiveLoader::load(const std::string & resour } } -std::unordered_map CLodArchiveLoader::getEntries() const +boost::unordered_map CLodArchiveLoader::getEntries() const { - std::unordered_map retList; + boost::unordered_map retList; for(auto it = entries.begin(); it != entries.end(); ++it) { @@ -194,7 +199,7 @@ std::unordered_map CLodArchiveLoader::getEntries() cons retList[ResourceID(entry.name)] = entry.name; } - return std::move(retList); + return retList; } const ArchiveEntry * CLodArchiveLoader::getArchiveEntry(const std::string & resourceName) const diff --git a/lib/Filesystem/CLodArchiveLoader.h b/lib/Filesystem/CLodArchiveLoader.h index 29e0d3e1a..2c1c7c61c 100644 --- a/lib/Filesystem/CLodArchiveLoader.h +++ b/lib/Filesystem/CLodArchiveLoader.h @@ -72,7 +72,7 @@ public: * * @return a list of all entries in the archive. */ - std::unordered_map getEntries() const; + boost::unordered_map getEntries() const; /** * Gets the archive entry for the requested resource @@ -122,5 +122,5 @@ private: std::string archive; /** Holds all entries of the archive file. An entry can be accessed via the entry name. **/ - std::unordered_map entries; + boost::unordered_map entries; }; diff --git a/lib/Filesystem/CResourceLoader.cpp b/lib/Filesystem/CResourceLoader.cpp index fa2601675..16274fd9b 100644 --- a/lib/Filesystem/CResourceLoader.cpp +++ b/lib/Filesystem/CResourceLoader.cpp @@ -8,6 +8,7 @@ #include "../JsonNode.h" #include "../GameConstants.h" #include "../VCMIDirs.h" +#include "../CStopWatch.h" CResourceLoader * CResourceHandler::resourceLoader = nullptr; CResourceLoader * CResourceHandler::initialLoader = nullptr; @@ -17,16 +18,16 @@ ResourceID::ResourceID() { } -ResourceID::ResourceID(const std::string & name) +ResourceID::ResourceID(std::string name) { - CFileInfo info(name); + CFileInfo info(std::move(name)); setName(info.getStem()); setType(info.getType()); } -ResourceID::ResourceID(const std::string & name, EResType::Type type) +ResourceID::ResourceID(std::string name, EResType::Type type) { - setName(name); + setName(std::move(name)); setType(type); } @@ -53,9 +54,9 @@ EResType::Type ResourceID::getType() const return type; } -void ResourceID::setName(const std::string & name) +void ResourceID::setName(std::string name) { - this->name = name; + this->name = std::move(name); size_t dotPos = this->name.find_last_of("/."); @@ -111,9 +112,9 @@ ResourceLocator CResourceLoader::getResource(const ResourceID & resourceIdent) c return resource->second.back(); } -const std::list & CResourceLoader::getResourcesWithName(const ResourceID & resourceIdent) const +const std::vector & CResourceLoader::getResourcesWithName(const ResourceID & resourceIdent) const { - static const std::list emptyList; + static const std::vector emptyList; auto resource = resources.find(resourceIdent); if (resource == resources.end()) @@ -163,7 +164,7 @@ void CResourceLoader::addLoader(std::string mountPoint, shared_ptr & entries = loader->getEntries(); + const boost::unordered_map & entries = loader->getEntries(); boost::to_upper(mountPoint); @@ -337,7 +338,8 @@ void CResourceHandler::loadFileSystem(const std::string fsConfigURI) { BOOST_FOREACH(auto & entry, mountPoint.second.Vector()) { - tlog5 << "loading resource at " << entry["path"].String() << "\n"; + CStopWatch timer; + tlog5 << "\t\tLoading resource at " << entry["path"].String(); std::string URI = entry["path"].String(); if (entry["type"].String() == "dir") @@ -364,6 +366,8 @@ void CResourceHandler::loadFileSystem(const std::string fsConfigURI) resourceLoader->addLoader(mountPoint.first, shared_ptr(new CLodArchiveLoader(filename)), false); } + + tlog5 << " took " << timer.getDiff() << " ms.\n"; } } } diff --git a/lib/Filesystem/CResourceLoader.h b/lib/Filesystem/CResourceLoader.h index 39f75cbba..b1ffb6e2d 100644 --- a/lib/Filesystem/CResourceLoader.h +++ b/lib/Filesystem/CResourceLoader.h @@ -71,12 +71,21 @@ public: */ ResourceID(); + /** + * Move Ctor. + */ + ResourceID(ResourceID && other) + : name(std::move(other.name)), type(other.getType()) + { + + } + /** * Ctor. Can be used to create indentifier for resource loading using one parameter * * @param name The resource name including extension. */ - explicit ResourceID(const std::string & fullName); + explicit ResourceID(std::string fullName); /** * Ctor. @@ -84,7 +93,7 @@ public: * @param name The resource name. * @param type The resource type. A constant from the enumeration EResType. */ - ResourceID(const std::string & name, EResType::Type type); + ResourceID(std::string name, EResType::Type type); /** * Compares this object with a another resource identifier. @@ -97,6 +106,16 @@ public: return name == other.name && type == other.type; } + /* + * Move-assignment operator. + */ + inline ResourceID& operator=(ResourceID && other) + { + name = std::move(other.name); + type = other.getType(); + return *this; + } + /** * Gets the name of the identifier. * @@ -116,7 +135,7 @@ public: * * @param name the name of the identifier. No extension, will be converted to uppercase. */ - void setName(const std::string & name); + void setName(std::string name); /** * Sets the type of the identifier. @@ -147,27 +166,40 @@ private: EResType::Type type; }; -namespace std +/** + * Generates a hash value for the resource identifier object. + * + * @param resourceIdent The object from which a hash value should be generated. + * @return the generated hash value + */ +inline size_t hash_value(const ResourceID & resourceIdent) { - /** - * Template specialization for std::hash. - */ - template <> - class hash - { -public: - /** - * Generates a hash value for the resource identifier object. - * - * @param resourceIdent The object from which a hash value should be generated. - * @return the generated hash value - */ - size_t operator()(const ResourceID & resourceIdent) const - { - return hash()(resourceIdent.getName()) ^ hash()(static_cast(resourceIdent.getType())); - } - }; -}; + boost::hash intHasher; + boost::hash stringHasher; + return stringHasher(resourceIdent.getName()) ^ intHasher(static_cast(resourceIdent.getType())); +} + +// namespace std +// { +// /** +// * Template specialization for std::hash. +// */ +// template <> +// class hash +// { +// public: +// /** +// * Generates a hash value for the resource identifier object. +// * +// * @param resourceIdent The object from which a hash value should be generated. +// * @return the generated hash value +// */ +// size_t operator()(const ResourceID & resourceIdent) const +// { +// return hash()(resourceIdent.getName()) ^ hash()(static_cast(resourceIdent.getType())); +// } +// }; +// }; /** * This class manages the loading of resources whether standard @@ -175,7 +207,7 @@ public: */ class DLL_LINKAGE CResourceLoader { - typedef std::unordered_map > ResourcesMap; + typedef boost::unordered_map > ResourcesMap; public: /// class for iterating over all available files/Identifiers @@ -259,7 +291,7 @@ public: ResourceLocator getResource(const ResourceID & resourceIdent) const; /// returns ALL overriden resources with same name, including last one acessible via getResource - const std::list & getResourcesWithName(const ResourceID & resourceIdent) const; + const std::vector & getResourcesWithName(const ResourceID & resourceIdent) const; /// returns real name of file in filesystem. Not usable for archives std::string getResourceName(const ResourceID & resourceIdent) const; @@ -309,7 +341,7 @@ public: */ void addLoader(std::string mountPoint, shared_ptr loader, bool writeable); -private: + public: /** * Contains lists of same resources which can be accessed uniquely by an diff --git a/lib/Filesystem/ISimpleResourceLoader.h b/lib/Filesystem/ISimpleResourceLoader.h index 3d9bf9884..00fd5342c 100644 --- a/lib/Filesystem/ISimpleResourceLoader.h +++ b/lib/Filesystem/ISimpleResourceLoader.h @@ -45,7 +45,7 @@ public: * * @return Returns a list of all entries in the archive or (file) system. */ - virtual std::unordered_map getEntries() const =0; + virtual boost::unordered_map getEntries() const =0; /** * Gets the origin of the loader.