[cpp-tests] Fileutils performance issue fix (#19231)

PatriceJiang · web-flow · commit 53d8b68fd028 · 2018-12-07T11:49:06.000+08:00
-   rename getFullPathForDirectoryAndFilename to getFullPathForFilenameWithinDirectory
-   add fullPathForDirectory, seperated from fullPathForFilename
-   add test case for FileUtils::listFiles
diff --git a/cocos/platform/CCFileUtils.cpp b/cocos/platform/CCFileUtils.cpp
@@ -808,7 +808,7 @@ std::string FileUtils::getPathForFilename(const std::string& filename, const std
     path += file_path;
     path += resolutionDirectory;
 
-    path = getFullPathForDirectoryAndFilename(path, file);
+    path = getFullPathForFilenameWithinDirectory(path, file);
 
     return path;
 }
@@ -864,6 +864,60 @@ std::string FileUtils::fullPathForFilename(const std::string &filename) const
     return "";
 }
 
+
+std::string FileUtils::fullPathForDirectory(const std::string &dir) const
+{
+    DECLARE_GUARD;
+
+    if (dir.empty())
+    {
+        return "";
+    }
+
+    if (isAbsolutePath(dir))
+    {
+        return dir;
+    }
+
+    // Already Cached ?
+    auto cacheIter = _fullPathCacheDir.find(dir);
+    if(cacheIter != _fullPathCacheDir.end())
+    {
+        return cacheIter->second;
+    }
+    std::string longdir = dir;
+    std::string fullpath;
+
+    if(longdir[longdir.length() - 1] != '/')
+    {
+        longdir +="/";
+    }
+
+    for (const auto& searchIt : _searchPathArray)
+    {
+        for (const auto& resolutionIt : _searchResolutionsOrderArray)
+        {
+            fullpath = searchIt + longdir + resolutionIt;
+            auto exists = isDirectoryExistInternal(fullpath);
+
+            if (exists && !fullpath.empty())
+            {
+                // Using the filename passed in as key.
+                _fullPathCacheDir.emplace(dir, fullpath);
+                return fullpath;
+            }
+
+        }
+    }
+
+    if(isPopupNotify()){
+        CCLOG("cocos2d: fullPathForDirectory: No directory found at %s. Possible missing directory.", dir.c_str());
+    }
+
+    // The file wasn't found, return empty string.
+    return "";
+}
+
 std::string FileUtils::fullPathFromRelativeFile(const std::string &filename, const std::string &relativeFile) const
 {
     return relativeFile.substr(0, relativeFile.rfind('/')+1) + getNewFilename(filename);
@@ -1053,7 +1107,7 @@ void FileUtils::loadFilenameLookupDictionaryFromFile(const std::string &filename
     }
 }
 
-std::string FileUtils::getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const
+std::string FileUtils::getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const
 {
     // get directory+filename, safely adding '/' as necessary
     std::string ret = directory;
@@ -1063,7 +1117,7 @@ std::string FileUtils::getFullPathForDirectoryAndFilename(const std::string& dir
     ret += filename;
 
     // if the file doesn't exist, return an empty string
-    if (!isFileExistInternal(ret) && !isDirectoryExistInternal(ret)) {
+    if (!isFileExistInternal(ret)) {
         ret = "";
     }
     return ret;
@@ -1122,7 +1176,7 @@ bool FileUtils::isDirectoryExist(const std::string& dirPath) const
         for (const auto& resolutionIt : _searchResolutionsOrderArray)
         {
             // searchPath + file_path + resourceDirectory
-            fullpath = fullPathForFilename(searchIt + dirPath + resolutionIt);
+            fullpath = fullPathForDirectory(searchIt + dirPath + resolutionIt);
             if (isDirectoryExistInternal(fullpath))
             {
                 _fullPathCache.emplace(dirPath, fullpath);
@@ -1188,15 +1242,15 @@ void FileUtils::getFileSize(const std::string &filepath, std::function<void(long
 
 void FileUtils::listFilesAsync(const std::string& dirPath, std::function<void(std::vector<std::string>)> callback) const
 {
-    auto fullPath = fullPathForFilename(dirPath);
+    auto fullPath = fullPathForDirectory(dirPath);
     performOperationOffthread([fullPath]() {
         return FileUtils::getInstance()->listFiles(fullPath);
     }, std::move(callback));
 }
 
 void FileUtils::listFilesRecursivelyAsync(const std::string& dirPath, std::function<void(std::vector<std::string>)> callback) const
 {
-    auto fullPath = fullPathForFilename(dirPath);
+    auto fullPath = fullPathForDirectory(dirPath);
     performOperationOffthread([fullPath]() {
         std::vector<std::string> retval;
         FileUtils::getInstance()->listFilesRecursively(fullPath, &retval);
@@ -1459,7 +1513,7 @@ long FileUtils::getFileSize(const std::string &filepath) const
 std::vector<std::string> FileUtils::listFiles(const std::string& dirPath) const
 {
     std::vector<std::string> files;
-    std::string fullpath = fullPathForFilename(dirPath);
+    std::string fullpath = fullPathForDirectory(dirPath);
     if (isDirectoryExist(fullpath))
     {
         tinydir_dir dir;
@@ -1497,7 +1551,7 @@ std::vector<std::string> FileUtils::listFiles(const std::string& dirPath) const
 
 void FileUtils::listFilesRecursively(const std::string& dirPath, std::vector<std::string> *files) const
 {
-    std::string fullpath = fullPathForFilename(dirPath);
+    std::string fullpath = fullPathForDirectory(dirPath);
     if (isDirectoryExist(fullpath))
     {
         tinydir_dir dir;
diff --git a/cocos/platform/CCFileUtils.h b/cocos/platform/CCFileUtils.h
@@ -343,6 +343,7 @@ class CC_DLL FileUtils
      */
     virtual std::string fullPathForFilename(const std::string &filename) const;
 
+
     /**
      * Loads the filenameLookup dictionary from the contents of a filename.
      *
@@ -905,8 +906,14 @@ class CC_DLL FileUtils
      *  @param filename  The name of the file.
      *  @return The full path of the file, if the file can't be found, it will return an empty string.
      */
-    virtual std::string getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const;
+    virtual std::string getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const;
+
 
+    /**
+     * Returns the fullpath for a given dirname.
+     * @since 3.17.1
+     */
+    virtual std::string fullPathForDirectory(const std::string &dirname) const;
 
     /**
     * mutex used to protect fields. 
@@ -950,11 +957,17 @@ class CC_DLL FileUtils
     std::string _defaultResRootPath;
 
     /**
-     *  The full path cache. When a file is found, it will be added into this cache.
+     *  The full path cache for normal files. When a file is found, it will be added into this cache.
      *  This variable is used for improving the performance of file search.
      */
     mutable std::unordered_map<std::string, std::string> _fullPathCache;
 
+    /**
+     *  The full path cache for directories. When a diretory is found, it will be added into this cache.
+     *  This variable is used for improving the performance of file search.
+     */
+    mutable std::unordered_map<std::string, std::string> _fullPathCacheDir;
+
     /**
      * Writable path.
      */
diff --git a/cocos/platform/android/CCFileUtils-android.cpp b/cocos/platform/android/CCFileUtils-android.cpp
@@ -227,13 +227,16 @@ bool FileUtilsAndroid::isDirectoryExistInternal(const std::string& dirPath) cons
     }
     else
     {
+
+
         // find it in apk's assets dir
         // Found "assets/" at the beginning of the path and we don't want it
         //CCLOG("find in apk dirPath(%s)", s);
         if (dirPath.find(ASSETS_FOLDER_NAME) == 0)
         {
             s += ASSETS_FOLDER_NAME_LENGTH;
         }
+
         if (FileUtilsAndroid::assetmanager)
         {
             AAssetDir* aa = AAssetManager_openDir(FileUtilsAndroid::assetmanager, s);
@@ -295,7 +298,7 @@ std::vector<std::string> FileUtilsAndroid::listFiles(const std::string& dirPath)
     if(isAbsolutePath(dirPath)) return FileUtils::listFiles(dirPath);
 
     std::vector<std::string> fileList;
-    string fullPath = fullPathForFilename(dirPath);
+    string fullPath = fullPathForDirectory(dirPath);
 
     static const std::string apkprefix("assets/");
     string relativePath = "";
diff --git a/cocos/platform/android/CCFileUtils-android.h b/cocos/platform/android/CCFileUtils-android.h
@@ -34,6 +34,8 @@ Copyright (c) 2017-2018 Xiamen Yaji Software Co., Ltd.
 #include "base/ccTypes.h"
 #include <string>
 #include <vector>
+#include <unordered_map>
+#include <memory>
 #include "jni.h"
 #include "android/asset_manager.h"
 
diff --git a/cocos/platform/apple/CCFileUtils-apple.h b/cocos/platform/apple/CCFileUtils-apple.h
@@ -50,7 +50,7 @@ class CC_DLL FileUtilsApple : public FileUtils
     virtual ~FileUtilsApple();
     /* override functions */
     virtual std::string getWritablePath() const override;
-    virtual std::string getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const override;
+    virtual std::string getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const override;
 
     virtual ValueMap getValueMapFromFile(const std::string& filename) const override;
     virtual ValueMap getValueMapFromData(const char* filedata, int filesize) const override;
diff --git a/cocos/platform/apple/CCFileUtils-apple.mm b/cocos/platform/apple/CCFileUtils-apple.mm
@@ -315,7 +315,7 @@ static int unlink_cb(const char *fpath, const struct stat *sb, int typeflag, str
         return true;
 }
 
-std::string FileUtilsApple::getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const
+std::string FileUtilsApple::getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const
 {
     if (directory[0] != '/')
     {
diff --git a/cocos/platform/win32/CCFileUtils-win32.cpp b/cocos/platform/win32/CCFileUtils-win32.cpp
@@ -220,12 +220,12 @@ std::string FileUtilsWin32::getPathForFilename(const std::string& filename, cons
     return FileUtils::getPathForFilename(unixFileName, unixResolutionDirectory, unixSearchPath);
 }
 
-std::string FileUtilsWin32::getFullPathForDirectoryAndFilename(const std::string& strDirectory, const std::string& strFilename) const
+std::string FileUtilsWin32::getFullPathForFilenameWithinDirectory(const std::string& strDirectory, const std::string& strFilename) const
 {
     std::string unixDirectory = convertPathFormatToUnixStyle(strDirectory);
     std::string unixFilename = convertPathFormatToUnixStyle(strFilename);
 
-    return FileUtils::getFullPathForDirectoryAndFilename(unixDirectory, unixFilename);
+    return FileUtils::getFullPathForFilenameWithinDirectory(unixDirectory, unixFilename);
 }
 
 void FileUtilsWin32::listFilesRecursively(const std::string& dirPath, std::vector<std::string> *files) const
@@ -286,7 +286,7 @@ long FileUtilsWin32::getFileSize(const std::string &filepath) const
 
 std::vector<std::string> FileUtilsWin32::listFiles(const std::string& dirPath) const
 {
-    std::string fullpath = fullPathForFilename(dirPath);
+    std::string fullpath = fullPathForDirectory(dirPath);
     std::vector<std::string> files;
     if (isDirectoryExist(fullpath))
     {
diff --git a/cocos/platform/win32/CCFileUtils-win32.h b/cocos/platform/win32/CCFileUtils-win32.h
@@ -134,7 +134,7 @@ class CC_DLL FileUtilsWin32 : public FileUtils
      *  @param filename  The name of the file.
      *  @return The full path of the file, if the file can't be found, it will return an empty string.
      */
-    virtual std::string getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const override;
+    virtual std::string getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const override;
 
     /**
     *  List all files in a directory.
diff --git a/cocos/platform/winrt/CCFileUtilsWinRT.cpp b/cocos/platform/winrt/CCFileUtilsWinRT.cpp
@@ -119,11 +119,11 @@ std::string CCFileUtilsWinRT::getPathForFilename(const std::string& filename, co
     return FileUtils::getPathForFilename(unixFileName, unixResolutionDirectory, unixSearchPath);
 }
 
-std::string CCFileUtilsWinRT::getFullPathForDirectoryAndFilename(const std::string& strDirectory, const std::string& strFilename) const
+std::string CCFileUtilsWinRT::getFullPathForFilenameWithinDirectory(const std::string& strDirectory, const std::string& strFilename) const
 {
     std::string unixDirectory = convertPathFormatToUnixStyle(strDirectory);
     std::string unixFilename = convertPathFormatToUnixStyle(strFilename);
-    return FileUtils::getFullPathForDirectoryAndFilename(unixDirectory, unixFilename);
+    return FileUtils::getFullPathForFilenameWithinDirectory(unixDirectory, unixFilename);
 }
 
 std::string CCFileUtilsWinRT::getSuitableFOpen(const std::string& filenameUtf8) const
@@ -380,7 +380,7 @@ string CCFileUtilsWinRT::getWritablePath() const
 
 void CCFileUtilsWinRT::listFilesRecursively(const std::string& dirPath, std::vector<std::string> *files) const
 {
-    std::string fullpath = fullPathForFilename(dirPath);
+    std::string fullpath = fullPathForDirectory(dirPath);
     if (isDirectoryExist(fullpath))
     {
         tinydir_dir dir;
@@ -426,7 +426,7 @@ void CCFileUtilsWinRT::listFilesRecursively(const std::string& dirPath, std::vec
 
 std::vector<std::string> CCFileUtilsWinRT::listFiles(const std::string& dirPath) const
 {
-    std::string fullpath = fullPathForFilename(dirPath);
+    std::string fullpath = fullPathForDirectory(dirPath);
     std::vector<std::string> files;
     if (isDirectoryExist(fullpath))
     {
diff --git a/cocos/platform/winrt/CCFileUtilsWinRT.h b/cocos/platform/winrt/CCFileUtilsWinRT.h
@@ -52,7 +52,7 @@ class CC_DLL CCFileUtilsWinRT : public FileUtils
     virtual std::string getWritablePath() const;
     virtual bool isAbsolutePath(const std::string& strPath) const;
     virtual std::string getPathForFilename(const std::string& filename, const std::string& resolutionDirectory, const std::string& searchPath) const override;
-    virtual std::string getFullPathForDirectoryAndFilename(const std::string& strDirectory, const std::string& strFilename) const override;
+    virtual std::string getFullPathForFilenameWithinDirectory(const std::string& strDirectory, const std::string& strFilename) const override;
     virtual std::string getSuitableFOpen(const std::string& filenameUtf8) const override;
     virtual long getFileSize(const std::string &filepath) override;
 	virtual FileUtils::Status getContents(const std::string& filename, ResizableBuffer* buffer) override;
diff --git a/tests/cpp-tests/Classes/FileUtilsTest/FileUtilsTest.cpp b/tests/cpp-tests/Classes/FileUtilsTest/FileUtilsTest.cpp
@@ -47,6 +47,7 @@ FileUtilsTests::FileUtilsTests()
     ADD_TEST_CASE(TestFileFuncsAsync);
     ADD_TEST_CASE(TestWriteStringAsync);
     ADD_TEST_CASE(TestWriteDataAsync);
+    ADD_TEST_CASE(TestListFiles);
 }
 
 // TestResolutionDirectories
@@ -1391,3 +1392,40 @@ std::string TestWriteDataAsync::subtitle() const
 {
     return "";
 }
+
+void TestListFiles::onEnter()
+{
+    FileUtilsDemo::onEnter();
+
+    auto winSize = Director::getInstance()->getWinSize();
+
+    auto infoLabel = Label::createWithTTF("show file count, should not be 0", "fonts/Thonburi.ttf", 18);
+    this->addChild(infoLabel);
+    infoLabel->setPosition(winSize.width / 2, winSize.height * 3 / 4);
+
+    auto cntLabel = Label::createWithTTF("show readResult", "fonts/Thonburi.ttf", 18);
+    this->addChild(cntLabel);
+    cntLabel->setPosition(winSize.width / 2, winSize.height / 3);
+    // writeTest
+    auto list = FileUtils::getInstance()->listFiles("fonts");
+
+    char cntBuffer[200] = { 0 };
+    snprintf(cntBuffer, 200, "%d", list.size());
+    cntLabel->setString(cntBuffer);
+
+}
+
+void TestListFiles::onExit()
+{
+    FileUtilsDemo::onExit();
+}
+
+std::string TestListFiles::title() const
+{
+    return "FileUtils: list files of directory";
+}
+
+std::string TestListFiles::subtitle() const
+{
+    return "";
+}
diff --git a/tests/cpp-tests/Classes/FileUtilsTest/FileUtilsTest.h b/tests/cpp-tests/Classes/FileUtilsTest/FileUtilsTest.h
@@ -258,4 +258,15 @@ class TestWriteDataAsync : public FileUtilsDemo
     virtual std::string subtitle() const override;
 };
 
+class TestListFiles : public FileUtilsDemo
+{
+public:
+    CREATE_FUNC(TestListFiles);
+
+    virtual void onEnter() override;
+    virtual void onExit() override;
+    virtual std::string title() const override;
+    virtual std::string subtitle() const override;
+};
+
 #endif /* __FILEUTILSTEST_H__ */

Original file line number	Diff line number	Diff line change
`@@ -227,13 +227,16 @@ bool FileUtilsAndroid::isDirectoryExistInternal(const std::string& dirPath) cons`
`227`	`227`	`}`
`228`	`228`	`else`
`229`	`229`	`{`
	`230`	`+`
	`231`	`+`
`230`	`232`	`// find it in apk's assets dir`
`231`	`233`	`// Found "assets/" at the beginning of the path and we don't want it`
`232`	`234`	`//CCLOG("find in apk dirPath(%s)", s);`
`233`	`235`	`if (dirPath.find(ASSETS_FOLDER_NAME) == 0)`
`234`	`236`	`{`
`235`	`237`	`s += ASSETS_FOLDER_NAME_LENGTH;`
`236`	`238`	`}`
	`239`	`+`
`237`	`240`	`if (FileUtilsAndroid::assetmanager)`
`238`	`241`	`{`
`239`	`242`	`AAssetDir* aa = AAssetManager_openDir(FileUtilsAndroid::assetmanager, s);`
`@@ -295,7 +298,7 @@ std::vector<std::string> FileUtilsAndroid::listFiles(const std::string& dirPath)`
`295`	`298`	`if(isAbsolutePath(dirPath)) return FileUtils::listFiles(dirPath);`
`296`	`299`
`297`	`300`	`std::vector<std::string> fileList;`
`298`		`- string fullPath = fullPathForFilename(dirPath);`
	`301`	`+ string fullPath = fullPathForDirectory(dirPath);`
`299`	`302`
`300`	`303`	`static const std::string apkprefix("assets/");`
`301`	`304`	`string relativePath = "";`
Original file line number	Diff line number	Diff line change
`@@ -315,7 +315,7 @@ static int unlink_cb(const char fpath, const struct stat sb, int typeflag, str`
`315`	`315`	`return true;`
`316`	`316`	`}`
`317`	`317`
`318`		`-std::string FileUtilsApple::getFullPathForDirectoryAndFilename(const std::string& directory, const std::string& filename) const`
	`318`	`+std::string FileUtilsApple::getFullPathForFilenameWithinDirectory(const std::string& directory, const std::string& filename) const`
`319`	`319`	`{`
`320`	`320`	`if (directory[0] != '/')`
`321`	`321`	`{`
Original file line number	Diff line number	Diff line change
`@@ -220,12 +220,12 @@ std::string FileUtilsWin32::getPathForFilename(const std::string& filename, cons`
`220`	`220`	`return FileUtils::getPathForFilename(unixFileName, unixResolutionDirectory, unixSearchPath);`
`221`	`221`	`}`
`222`	`222`
`223`		`-std::string FileUtilsWin32::getFullPathForDirectoryAndFilename(const std::string& strDirectory, const std::string& strFilename) const`
	`223`	`+std::string FileUtilsWin32::getFullPathForFilenameWithinDirectory(const std::string& strDirectory, const std::string& strFilename) const`
`224`	`224`	`{`
`225`	`225`	`std::string unixDirectory = convertPathFormatToUnixStyle(strDirectory);`
`226`	`226`	`std::string unixFilename = convertPathFormatToUnixStyle(strFilename);`
`227`	`227`
`228`		`- return FileUtils::getFullPathForDirectoryAndFilename(unixDirectory, unixFilename);`
	`228`	`+ return FileUtils::getFullPathForFilenameWithinDirectory(unixDirectory, unixFilename);`
`229`	`229`	`}`
`230`	`230`
`231`	`231`	`void FileUtilsWin32::listFilesRecursively(const std::string& dirPath, std::vector<std::string> *files) const`
`@@ -286,7 +286,7 @@ long FileUtilsWin32::getFileSize(const std::string &filepath) const`
`286`	`286`
`287`	`287`	`std::vector<std::string> FileUtilsWin32::listFiles(const std::string& dirPath) const`
`288`	`288`	`{`
`289`		`- std::string fullpath = fullPathForFilename(dirPath);`
	`289`	`+ std::string fullpath = fullPathForDirectory(dirPath);`
`290`	`290`	`std::vector<std::string> files;`
`291`	`291`	`if (isDirectoryExist(fullpath))`
`292`	`292`	`{`
Original file line number	Diff line number	Diff line change
`@@ -119,11 +119,11 @@ std::string CCFileUtilsWinRT::getPathForFilename(const std::string& filename, co`
`119`	`119`	`return FileUtils::getPathForFilename(unixFileName, unixResolutionDirectory, unixSearchPath);`
`120`	`120`	`}`
`121`	`121`
`122`		`-std::string CCFileUtilsWinRT::getFullPathForDirectoryAndFilename(const std::string& strDirectory, const std::string& strFilename) const`
	`122`	`+std::string CCFileUtilsWinRT::getFullPathForFilenameWithinDirectory(const std::string& strDirectory, const std::string& strFilename) const`
`123`	`123`	`{`
`124`	`124`	`std::string unixDirectory = convertPathFormatToUnixStyle(strDirectory);`
`125`	`125`	`std::string unixFilename = convertPathFormatToUnixStyle(strFilename);`
`126`		`- return FileUtils::getFullPathForDirectoryAndFilename(unixDirectory, unixFilename);`
	`126`	`+ return FileUtils::getFullPathForFilenameWithinDirectory(unixDirectory, unixFilename);`
`127`	`127`	`}`
`128`	`128`
`129`	`129`	`std::string CCFileUtilsWinRT::getSuitableFOpen(const std::string& filenameUtf8) const`
`@@ -380,7 +380,7 @@ string CCFileUtilsWinRT::getWritablePath() const`
`380`	`380`
`381`	`381`	`void CCFileUtilsWinRT::listFilesRecursively(const std::string& dirPath, std::vector<std::string> *files) const`
`382`	`382`	`{`
`383`		`- std::string fullpath = fullPathForFilename(dirPath);`
	`383`	`+ std::string fullpath = fullPathForDirectory(dirPath);`
`384`	`384`	`if (isDirectoryExist(fullpath))`
`385`	`385`	`{`
`386`	`386`	`tinydir_dir dir;`
`@@ -426,7 +426,7 @@ void CCFileUtilsWinRT::listFilesRecursively(const std::string& dirPath, std::vec`
`426`	`426`
`427`	`427`	`std::vector<std::string> CCFileUtilsWinRT::listFiles(const std::string& dirPath) const`
`428`	`428`	`{`
`429`		`- std::string fullpath = fullPathForFilename(dirPath);`
	`429`	`+ std::string fullpath = fullPathForDirectory(dirPath);`
`430`	`430`	`std::vector<std::string> files;`
`431`	`431`	`if (isDirectoryExist(fullpath))`
`432`	`432`	`{`