birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[development] Fixed runaway memory in editor (#2220)

Browse Source 
There was a change in bahaviour to how files/directories were collected in the AZ::IO::FindData::Scan* functions which allowed the addition of duplicate entries. This created a problem when attempting a recursively greedy search with zips included because of the level system pak files. Their mount points would be the root "levels" folder, so for N-levels there would be N identical entries to the "levels" folder being added perpetually each time ScanZips was called on the "levels" folder or above folders with "*" filtering.

Letting the editor sit idle no longer sees the reported memory in the bottom status bar climb. Also, running the tests which prompted the initial change shows a negligible change in perf.

Signed-off-by: AMZN-ScottR 24445312+AMZN-ScottR@users.noreply.github.com
This commit is contained in:
Scott Romero
2021-07-19 16:14:41 -07:00
committed by GitHub
parent 4f76838782
commit 3c378b348c
3 changed files with 38 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzFramework/AzFramework/Archive/ArchiveFindData.cpp
+24 -11
View File
@@ -15,6 +15,11 @@
namespace AZ::IO
{
size_t ArchiveFileIteratorHash::operator()(const AZ::IO::ArchiveFileIterator& iter) const
{
return iter.GetHash();
}
bool AZStdStringLessCaseInsensitive::operator()(AZStd::string_view left, AZStd::string_view right) const
{
// If one or both strings are 0-length, return true if the left side is smaller, false if they're equal or left is larger.
@@ -68,11 +73,21 @@ namespace AZ::IO
return operator++();
}
bool ArchiveFileIterator::operator==(const AZ::IO::ArchiveFileIterator& rhs) const
{
return GetHash() == rhs.GetHash();
}
ArchiveFileIterator::operator bool() const
{
return m_findData && m_lastFetchValid;
}
size_t ArchiveFileIterator::GetHash() const
{
return AZStd::hash<AZ::IO::PathView>{}(m_filename.c_str());
}
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
@@ -113,14 +128,12 @@ namespace AZ::IO
}
AZ::IO::FileIOBase::GetDirectInstance()->FindFiles(searchDirectory.c_str(), pattern.c_str(), [&](const char* filePath) -> bool
{
AZ::IO::ArchiveFileIterator fileIterator;
fileIterator.m_filename = AZ::IO::PathView(filePath).Filename().Native();
fileIterator.m_fileDesc.nAttrib = {};
AZ::IO::ArchiveFileIterator fileIterator{ nullptr, AZ::IO::PathView(filePath).Filename().Native(), {} };
if (AZ::IO::FileIOBase::GetDirectInstance()->IsDirectory(filePath))
{
fileIterator.m_fileDesc.nAttrib = fileIterator.m_fileDesc.nAttrib | AZ::IO::FileDesc::Attribute::Subdirectory;
m_fileStack.emplace_back(AZStd::move(fileIterator));
m_fileSet.emplace(AZStd::move(fileIterator));
}
else
{
@@ -136,7 +149,7 @@ namespace AZ::IO
// These times are not supported by our file interface
fileIterator.m_fileDesc.tAccess = fileIterator.m_fileDesc.tWrite;
fileIterator.m_fileDesc.tCreate = fileIterator.m_fileDesc.tWrite;
m_fileStack.emplace_back(AZStd::move(fileIterator));
m_fileSet.emplace(AZStd::move(fileIterator));
}
return true;
});
@@ -167,7 +180,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_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
m_fileSet.emplace(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
}
ZipDir::FindDir findDirectoryEntry(zipCache);
@@ -180,7 +193,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_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
m_fileSet.emplace(AZ::IO::ArchiveFileIterator{ this, fname, fileDesc });
}
};
@@ -249,7 +262,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_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ this, bindRootIter->Native(), fileDesc });
m_fileSet.emplace(AZ::IO::ArchiveFileIterator{ this, bindRootIter->Native(), fileDesc });
}
}
else
@@ -265,7 +278,7 @@ namespace AZ::IO
AZ::IO::ArchiveFileIterator FindData::Fetch()
{
if (m_fileStack.empty())
if (m_fileSet.empty())
{
AZ::IO::ArchiveFileIterator emptyFileIterator;
emptyFileIterator.m_lastFetchValid = false;
@@ -274,10 +287,10 @@ namespace AZ::IO
}
// Remove Fetched item from the FindData map so that the iteration continues
AZ::IO::ArchiveFileIterator fileIterator{ m_fileStack.back() };
AZ::IO::ArchiveFileIterator fileIterator{ *m_fileSet.begin() };
fileIterator.m_lastFetchValid = true;
fileIterator.m_findData = this;
m_fileStack.pop_back();
m_fileSet.erase(m_fileSet.begin());
return fileIterator;
}
}
Code/Framework/AzFramework/AzFramework/Archive/ArchiveFindData.h
+12 -2
View File
@@ -8,6 +8,7 @@
#pragma once
#include <AzCore/std/containers/unordered_set.h>
#include <AzCore/std/smart_ptr/intrusive_base.h>
#include <AzCore/std/string/fixed_string.h>
@@ -44,8 +45,12 @@ namespace AZ::IO
ArchiveFileIterator operator++();
ArchiveFileIterator operator++(int);
bool operator==(const AZ::IO::ArchiveFileIterator& rhs) const;
explicit operator bool() const;
size_t GetHash() const;
inline static constexpr size_t FilenameMaxLength = 256;
AZStd::fixed_string<FilenameMaxLength> m_filename;
FileDesc m_fileDesc;
@@ -56,6 +61,11 @@ namespace AZ::IO
bool m_lastFetchValid{};
};
struct ArchiveFileIteratorHash
{
size_t operator()(const AZ::IO::ArchiveFileIterator& iter) const;
};
struct AZStdStringLessCaseInsensitive
{
bool operator()(AZStd::string_view left, AZStd::string_view right) const;
@@ -75,7 +85,7 @@ namespace AZ::IO
void ScanFS(IArchive* archive, AZStd::string_view path);
void ScanZips(IArchive* archive, AZStd::string_view path);
using FileStack = AZStd::vector<ArchiveFileIterator>;
FileStack m_fileStack;
using FileSet = AZStd::unordered_set<ArchiveFileIterator, ArchiveFileIteratorHash>;
FileSet m_fileSet;
};
}
Gems/AudioSystem/Code/Tests/AudioSystemEditorTest.cpp
+2 -2
View File
@@ -38,7 +38,7 @@ namespace CustomMocks
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
m_findData = new TestFindData();
m_findData->m_fileStack.emplace_back(AZ::IO::ArchiveFileIterator{ static_cast<AZ::IO::FindData*>(m_findData.get()), m_levelName, fileDesc });
m_findData->m_fileSet.emplace(AZ::IO::ArchiveFileIterator{ static_cast<AZ::IO::FindData*>(m_findData.get()), m_levelName, fileDesc });
return m_findData->Fetch();
}
@@ -54,7 +54,7 @@ namespace CustomMocks
struct TestFindData
: AZ::IO::FindData
{
using AZ::IO::FindData::m_fileStack;
using AZ::IO::FindData::m_fileSet;
};
AZStd::intrusive_ptr<TestFindData> m_findData;