{LYN-4224} Fix for the file scan slowdown (#1252)

* {LYN-4224} Fix for the file scan slowdown (#1183)

* {LYN-4224} Fix for the file scan slowdown

* Fixed a slowdown in the file scanning logic
* Improved the file scanning logic from previous code by 40%

Tests:
Using Testing\Pytest\AutomatedTesting_BlastTest

old code:
=== 7 passed in 96.13s (0:01:36) ===

current code:
=== 7 passed in 160.45s (0:02:40) ====

newest code:
=== 7 passed in 52.91s ===

* fixing a unit test compile error

* unit test fixes

* another file improvement

* fix for legacy level loading taking too long

* making an enum for the search types

* switched the enum to "allow" types to make the input more clear

* got rid of orphaned const variables

Code/CryEngine/CryCommon/Mocks/ICryPakMock.h

@@ -67,7 +67,7 @@ struct CryPakMock
     MOCK_METHOD1(PoolMalloc, void*(size_t size));
     MOCK_METHOD1(PoolFree, void(void* p));
     MOCK_METHOD3(PoolAllocMemoryBlock, AZStd::intrusive_ptr<AZ::IO::MemoryBlock> (size_t nSize, const char* sUsage, size_t nAlign));
-    MOCK_METHOD3(FindFirst, AZ::IO::ArchiveFileIterator(AZStd::string_view pDir, uint32_t nFlags, bool bAllOwUseFileSystem));
+    MOCK_METHOD2(FindFirst, AZ::IO::ArchiveFileIterator(AZStd::string_view pDir, AZ::IO::IArchive::EFileSearchType));
     MOCK_METHOD1(FindNext, AZ::IO::ArchiveFileIterator(AZ::IO::ArchiveFileIterator handle));
     MOCK_METHOD1(FindClose, bool(AZ::IO::ArchiveFileIterator));
     MOCK_METHOD1(GetModificationTime, AZ::IO::IArchive::FileTime(AZ::IO::HandleType f));

Code/CryEngine/CrySystem/LevelSystem/LevelSystem.cpp

@@ -306,8 +306,7 @@ void CLevelSystem::ScanFolder(const char* subfolder, bool modFolder)
 
     AZStd::unordered_set<AZStd::string> pakList;
 
-    bool allowFileSystem = true;
-    AZ::IO::ArchiveFileIterator handle = pPak->FindFirst(search.c_str(), 0, allowFileSystem);
+    AZ::IO::ArchiveFileIterator handle = pPak->FindFirst(search.c_str(), AZ::IO::IArchive::eFileSearchType_AllowOnDiskOnly);
 
     if (handle)
     {
@@ -320,7 +319,7 @@ void CLevelSystem::ScanFolder(const char* subfolder, bool modFolder)
             {
                 if (AZ::StringFunc::Equal(handle.m_filename.data(), LevelPakName))
                 {
-                    // level folder contain pak files like 'level.pak' 
+                    // level folder contain pak files like 'level.pak'
                     // which we only want to load during level loading.
                     continue;
                 }
@@ -351,7 +350,7 @@ void CLevelSystem::ScanFolder(const char* subfolder, bool modFolder)
     PopulateLevels(search, folder, pPak, modFolder, false);
     // Load levels outside of the bundles to maintain backward compatibility.
     PopulateLevels(search, folder, pPak, modFolder, true);
-      
+
 }
 
 void CLevelSystem::PopulateLevels(
@@ -360,7 +359,7 @@ void CLevelSystem::PopulateLevels(
     {
         // allow this find first to actually touch the file system
         // (causes small overhead but with minimal amount of levels this should only be around 150ms on actual DVD Emu)
-        AZ::IO::ArchiveFileIterator handle = pPak->FindFirst(searchPattern.c_str(), 0, fromFileSystemOnly);
+        AZ::IO::ArchiveFileIterator handle = pPak->FindFirst(searchPattern.c_str(), AZ::IO::IArchive::eFileSearchType_AllowOnDiskOnly);
 
         if (handle)
         {
@@ -973,7 +972,7 @@ void CLevelSystem::UnloadLevel()
     m_lastLevelName.clear();
 
     SAFE_RELEASE(m_pCurrentLevel);
-    
+
     // Force Lua garbage collection (may no longer be needed now the legacy renderer has been removed).
     // Normally the GC step is triggered at the end of this method (by the ESYSTEM_EVENT_LEVEL_POST_UNLOAD event).
     EBUS_EVENT(AZ::ScriptSystemRequestBus, GarbageCollect);

Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp

@@ -1290,7 +1290,7 @@ namespace AZ::IO
 
 
     //////////////////////////////////////////////////////////////////////////
-    AZ::IO::ArchiveFileIterator Archive::FindFirst(AZStd::string_view pDir, [[maybe_unused]] uint32_t nPathFlags, bool bAllowUseFileSystem)
+    AZ::IO::ArchiveFileIterator Archive::FindFirst(AZStd::string_view pDir, EFileSearchType searchType)
     {
         auto szFullPath = AZ::IO::FileIOBase::GetDirectInstance()->ResolvePath(pDir);
         if (!szFullPath)
@@ -1299,8 +1299,26 @@ namespace AZ::IO
             return {};
         }
 
+        bool bScanZips{};
+        bool bAllowUseFileSystem{};
+        switch (searchType)
+        {
+            case IArchive::eFileSearchType_AllowInZipsOnly:
+                bAllowUseFileSystem = false;
+                bScanZips = true;
+                break;
+            case IArchive::eFileSearchType_AllowOnDiskAndInZips:
+                bAllowUseFileSystem = true;
+                bScanZips = true;
+                break;
+            case IArchive::eFileSearchType_AllowOnDiskOnly:
+                bAllowUseFileSystem = true;
+                bScanZips = false;
+                break;
+        }
+
         AZStd::intrusive_ptr<AZ::IO::FindData> pFindData = new AZ::IO::FindData();
-        pFindData->Scan(this, szFullPath->Native(), bAllowUseFileSystem);
+        pFindData->Scan(this, szFullPath->Native(), bAllowUseFileSystem, bScanZips);
 
         return pFindData->Fetch();
     }
@@ -1676,7 +1694,7 @@ namespace AZ::IO
             return true;
         }
 
-        if (AZ::IO::ArchiveFileIterator fileIterator = FindFirst(pWildcardIn, 0, true); fileIterator)
+        if (AZ::IO::ArchiveFileIterator fileIterator = FindFirst(pWildcardIn, IArchive::eFileSearchType_AllowOnDiskOnly); fileIterator)
         {
             AZStd::vector<AZStd::string> files;
             do

Code/Framework/AzFramework/AzFramework/Archive/Archive.h

@@ -234,7 +234,7 @@ namespace AZ::IO
         uint64_t FTell(AZ::IO::HandleType handle) override;
         int FFlush(AZ::IO::HandleType handle) override;
         int FClose(AZ::IO::HandleType handle) override;
-        AZ::IO::ArchiveFileIterator FindFirst(AZStd::string_view pDir, uint32_t nPathFlags = 0, bool bAllOwUseFileSystem = false) override;
+        AZ::IO::ArchiveFileIterator FindFirst(AZStd::string_view pDir, EFileSearchType searchType = eFileSearchType_AllowInZipsOnly) override;
         AZ::IO::ArchiveFileIterator FindNext(AZ::IO::ArchiveFileIterator fileIterator) override;
         bool FindClose(AZ::IO::ArchiveFileIterator fileIterator) override;
         int FEof(AZ::IO::HandleType handle) override;

Code/Framework/AzFramework/AzFramework/Archive/ArchiveFindData.cpp

@@ -77,7 +77,7 @@ namespace AZ::IO
         return m_findData && m_lastFetchValid;
     }
 
-    void FindData::Scan(IArchive* archive, AZStd::string_view szDir, bool bAllowUseFS)
+    void FindData::Scan(IArchive* archive, AZStd::string_view szDir, bool bAllowUseFS, bool bScanZips)
     {
         // get the priority into local variable to avoid it changing in the course of
         // this function execution
@@ -87,12 +87,18 @@ namespace AZ::IO
         {
             // first, find the file system files
             ScanFS(archive, szDir);
-            ScanZips(archive, szDir);
+            if (bScanZips)
+            {
+                ScanZips(archive, szDir);
+            }
         }
         else
         {
             // first, find the zip files
-            ScanZips(archive, szDir);
+            if (bScanZips)
+            {
+                ScanZips(archive, szDir);
+            }
             if (bAllowUseFS || nVarPakPriority != ArchiveLocationPriority::ePakPriorityPakOnly)
             {
                 ScanFS(archive, szDir);
@@ -111,30 +117,31 @@ namespace AZ::IO
         }
         AZ::IO::FileIOBase::GetDirectInstance()->FindFiles(searchDirectory.c_str(), pattern.c_str(), [&](const char* filePath) -> bool
         {
-            AZ::IO::FileDesc fileDesc;
-            AZStd::string filePathEntry{filePath};
+            AZ::IO::ArchiveFileIterator fileIterator;
+            fileIterator.m_filename = AZ::IO::PathView(filePath).Filename().Native();
+            fileIterator.m_fileDesc.nAttrib = {};
 
             if (AZ::IO::FileIOBase::GetDirectInstance()->IsDirectory(filePath))
             {
-                fileDesc.nAttrib = fileDesc.nAttrib | AZ::IO::FileDesc::Attribute::Subdirectory;
+                fileIterator.m_fileDesc.nAttrib = fileIterator.m_fileDesc.nAttrib | AZ::IO::FileDesc::Attribute::Subdirectory;
+                m_fileStack.emplace_back(AZStd::move(fileIterator));
             }
             else
             {
                 if (AZ::IO::FileIOBase::GetDirectInstance()->IsReadOnly(filePath))
                 {
-                    fileDesc.nAttrib = fileDesc.nAttrib | AZ::IO::FileDesc::Attribute::ReadOnly;
+                    fileIterator.m_fileDesc.nAttrib = fileIterator.m_fileDesc.nAttrib | AZ::IO::FileDesc::Attribute::ReadOnly;
                 }
                 AZ::u64 fileSize = 0;
                 AZ::IO::FileIOBase::GetDirectInstance()->Size(filePath, fileSize);
-                fileDesc.nSize = fileSize;
-                fileDesc.tWrite = AZ::IO::FileIOBase::GetDirectInstance()->ModificationTime(filePath);
+                fileIterator.m_fileDesc.nSize = fileSize;
+                fileIterator.m_fileDesc.tWrite = AZ::IO::FileIOBase::GetDirectInstance()->ModificationTime(filePath);
 
                 // These times are not supported by our file interface
-                fileDesc.tAccess = fileDesc.tWrite;
-                fileDesc.tCreate = fileDesc.tWrite;
+                fileIterator.m_fileDesc.tAccess = fileIterator.m_fileDesc.tWrite;
+                fileIterator.m_fileDesc.tCreate = fileIterator.m_fileDesc.tWrite;
+                m_fileStack.emplace_back(AZStd::move(fileIterator));
             }
-            [[maybe_unused]] auto result = m_mapFiles.emplace(AZStd::move(filePathEntry), fileDesc);
-            AZ_Assert(result.second, "Failed to insert FindData entry for filePath %s", filePath);
             return true;
         });
     }
@@ -164,7 +171,7 @@ namespace AZ::IO
                 fileDesc.nAttrib = AZ::IO::FileDesc::Attribute::ReadOnly | AZ::IO::FileDesc::Attribute::Archive;
                 fileDesc.nSize = fileEntry->desc.lSizeUncompressed;
                 fileDesc.tWrite = fileEntry->GetModificationTime();
-                m_mapFiles.emplace(fname, fileDesc);
+                m_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
             }
 
             ZipDir::FindDir findDirectoryEntry(zipCache);
@@ -177,7 +184,7 @@ namespace AZ::IO
                 }
                 AZ::IO::FileDesc fileDesc;
                 fileDesc.nAttrib = AZ::IO::FileDesc::Attribute::ReadOnly | AZ::IO::FileDesc::Attribute::Archive | AZ::IO::FileDesc::Attribute::Subdirectory;
-                m_mapFiles.emplace(fname, fileDesc);
+                m_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
             }
         };
 
@@ -246,7 +253,7 @@ namespace AZ::IO
                 if (!bindRootIter->empty() && AZStd::wildcard_match(sourcePathRemainder.Native(), bindRootIter->Native()))
                 {
                     AZ::IO::FileDesc fileDesc{ AZ::IO::FileDesc::Attribute::ReadOnly | AZ::IO::FileDesc::Attribute::Archive | AZ::IO::FileDesc::Attribute::Subdirectory };
-                    m_mapFiles.emplace(bindRootIter->Native(), fileDesc);
+                    m_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, bindRootIter->Native(), fileDesc });
                 }
             }
             else
@@ -262,22 +269,19 @@ namespace AZ::IO
 
     AZ::IO::ArchiveFileIterator FindData::Fetch()
     {
-        AZ::IO::ArchiveFileIterator fileIterator;
-        fileIterator.m_findData = this;
-        if (m_mapFiles.empty())
+        if (m_fileStack.empty())
         {
-            return fileIterator;
+            AZ::IO::ArchiveFileIterator emptyFileIterator;
+            emptyFileIterator.m_lastFetchValid = false;
+            emptyFileIterator.m_findData = this;
+            return emptyFileIterator;
         }
 
-        auto pakFileIter = m_mapFiles.begin();
-        AZStd::string fullFilePath;
-        AZ::StringFunc::Path::GetFullFileName(pakFileIter->first.c_str(), fullFilePath);
-        fileIterator.m_filename = AZStd::move(fullFilePath);
-        fileIterator.m_fileDesc = pakFileIter->second;
-        fileIterator.m_lastFetchValid = true;
-
         // Remove Fetched item from the FindData map so that the iteration continues
-        m_mapFiles.erase(pakFileIter);
+        AZ::IO::ArchiveFileIterator fileIterator{ m_fileStack.back() };
+        fileIterator.m_lastFetchValid = true;
+        fileIterator.m_findData = this;
+        m_fileStack.pop_back();
         return fileIterator;
     }
 }

Code/Framework/AzFramework/AzFramework/Archive/ArchiveFindData.h

@@ -15,7 +15,6 @@
 #include <AzCore/std/smart_ptr/intrusive_base.h>
 #include <AzCore/std/string/fixed_string.h>
 
-
 namespace AZ::IO
 {
     struct IArchive;
@@ -74,13 +73,13 @@ namespace AZ::IO
         AZ_CLASS_ALLOCATOR(FindData, AZ::SystemAllocator, 0);
         FindData() = default;
         AZ::IO::ArchiveFileIterator Fetch();
-        void Scan(IArchive* archive, AZStd::string_view path, bool bAllowUseFS = false);
+        void Scan(IArchive* archive, AZStd::string_view path, bool bAllowUseFS = false, bool bScanZips = true);
 
     protected:
         void ScanFS(IArchive* archive, AZStd::string_view path);
         void ScanZips(IArchive* archive, AZStd::string_view path);
 
-        using FileMap = AZStd::map<AZStd::string, AZ::IO::FileDesc, AZStdStringLessCaseInsensitive>;
-        FileMap m_mapFiles;
+        using FileStack = AZStd::vector<ArchiveFileIterator>;
+        FileStack m_fileStack;
     };
 }

Code/Framework/AzFramework/AzFramework/Archive/IArchive.h

@@ -197,6 +197,13 @@ namespace AZ::IO
             eInMemoryPakLocale_PAK,
         };
 
+        enum EFileSearchType
+        {
+            eFileSearchType_AllowInZipsOnly = 0,
+            eFileSearchType_AllowOnDiskAndInZips,
+            eFileSearchType_AllowOnDiskOnly
+        };
+
         using SignedFileSize = int64_t;
 
         virtual ~IArchive() = default;
@@ -315,7 +322,7 @@ namespace AZ::IO
 
         // Arguments:
         //   nFlags is a combination of EPathResolutionRules flags.
-        virtual ArchiveFileIterator FindFirst(AZStd::string_view pDir, uint32_t nFlags = 0, bool bAllowUseFileSystem = false) = 0;
+        virtual ArchiveFileIterator FindFirst(AZStd::string_view pDir, EFileSearchType searchType = eFileSearchType_AllowInZipsOnly) = 0;
         virtual ArchiveFileIterator FindNext(AZ::IO::ArchiveFileIterator handle) = 0;
         virtual bool FindClose(AZ::IO::ArchiveFileIterator handle) = 0;
         //returns file modification time

Code/Sandbox/Editor/CryEditDoc.cpp

@@ -1139,7 +1139,7 @@ bool CCryEditDoc::SaveLevel(const QString& filename)
         const QString oldLevelPattern = QDir(oldLevelFolder).absoluteFilePath("*.*");
         const QString oldLevelName = Path::GetFile(GetLevelPathName());
         const QString oldLevelXml = Path::ReplaceExtension(oldLevelName, "xml");
-        AZ::IO::ArchiveFileIterator findHandle = pIPak->FindFirst(oldLevelPattern.toUtf8().data(), 0, true);
+        AZ::IO::ArchiveFileIterator findHandle = pIPak->FindFirst(oldLevelPattern.toUtf8().data(), AZ::IO::IArchive::eFileSearchType_AllowOnDiskAndInZips);
         if (findHandle)
         {
             do

Gems/AudioSystem/Code/Tests/AudioSystemEditorTest.cpp

@@ -36,14 +36,14 @@ namespace CustomMocks
             : m_levelName(levelName)
         {}
 
-        AZ::IO::ArchiveFileIterator FindFirst([[maybe_unused]] AZStd::string_view dir, [[maybe_unused]] unsigned int flags, [[maybe_unused]] bool allowUseFileSystem) override
+        AZ::IO::ArchiveFileIterator FindFirst([[maybe_unused]] AZStd::string_view dir, AZ::IO::IArchive::EFileSearchType) override
         {
             AZ::IO::FileDesc fileDesc;
             fileDesc.nSize = sizeof(AZ::IO::FileDesc);
             // Add a filename and file description reference to the TestFindData map to make sure the file iterator is valid
-            AZStd::intrusive_ptr<TestFindData> findData = new TestFindData{};
-            findData->m_mapFiles.emplace(m_levelName, fileDesc);
-            return findData->Fetch();
+            m_findData = new TestFindData();
+            m_findData->m_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ static_cast<AZ::IO::FindData*>(m_findData.get()), m_levelName, fileDesc });
+            return m_findData->Fetch();
         }
 
         AZ::IO::ArchiveFileIterator FindNext(AZ::IO::ArchiveFileIterator iter) override
@@ -54,13 +54,14 @@ namespace CustomMocks
         // public: for easy resetting...
         AZStd::string m_levelName;
 
-
         // Add an inherited FindData class to control the adding of a mapfile which indicates that a FileIterator is valid
         struct TestFindData
             : AZ::IO::FindData
         {
-            using AZ::IO::FindData::m_mapFiles;
+            using AZ::IO::FindData::m_fileStack;
         };
+
+        AZStd::intrusive_ptr<TestFindData> m_findData;
     };
 
 } // namespace CustomMocks