Skip to content

Commit

Permalink
Improve destructor performance by 97% on very large folders
Browse files Browse the repository at this point in the history
- Use iter_swap/pop_back idiom to improve FolderData::removeChild()
  performance
- Re-order deletions to take advantage of iter_swap/pop_back idiom
- Add FolderData::bulkRemoveChildren() and use it for removal of
  virtual folders
- Prevent inefficient unlinking of child files from a folder being
  destroyed
  • Loading branch information
n2qz committed Nov 27, 2024
1 parent df7da26 commit 61b9844
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 22 deletions.
64 changes: 42 additions & 22 deletions es-app/src/FileData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,14 +1313,12 @@ void FolderData::removeChild(FileData* file)
assert(file->getParent() == this);
#endif

for (auto it = mChildren.cbegin(); it != mChildren.cend(); it++)
auto it = std::find(mChildren.begin(), mChildren.end(), file);
if (it != mChildren.end())
{
if (*it == file)
{
file->setParent(NULL);
mChildren.erase(it);
return;
}
file->setParent(nullptr);
std::iter_swap(it, mChildren.end() - 1);
mChildren.pop_back();
}

// File somehow wasn't in our children.
Expand All @@ -1329,6 +1327,26 @@ void FolderData::removeChild(FileData* file)
#endif
}

void FolderData::bulkRemoveChildren(std::vector<FileData*>& mChildren, const std::unordered_set<FileData*>& filesToRemove)
{
mChildren.erase(
std::remove_if(
mChildren.begin(),
mChildren.end(),
[&filesToRemove](FileData* file)
{
if (filesToRemove.count(file))
{
file->setParent(nullptr);
return true;
}
return false;
}
),
mChildren.end()
);
}

FileData* FolderData::FindByPath(const std::string& path)
{
std::vector<FileData*> children = getChildren();
Expand Down Expand Up @@ -1506,23 +1524,26 @@ void FileData::detectLanguageAndRegion(bool overWrite)
mMetadata.set(MetaDataId::Region, info.region);
}

void FolderData::removeVirtualFolders()
{
void FolderData::removeVirtualFolders() {
if (!mOwnsChildrens)
return;

for (int i = mChildren.size() - 1; i >= 0; i--)
std::unordered_set<FileData*> filesToRemove;

for (auto file : mChildren)
{
auto file = mChildren.at(i);
if (file->getType() != FOLDER)
continue;

if (((FolderData*)file)->mOwnsChildrens)
continue;
auto folder = static_cast<FolderData*>(file);
if (!folder->mOwnsChildrens)
filesToRemove.insert(file);
}

bulkRemoveChildren(mChildren, filesToRemove);

removeChild(file);
for (auto file : filesToRemove)
delete file;
}
}

void FileData::checkCrc32(bool force)
Expand Down Expand Up @@ -1726,14 +1747,13 @@ FolderData::~FolderData()
clear();
}

void FolderData::clear()
{
void FolderData::clear() {
if (mOwnsChildrens)
{
for (int i = mChildren.size() - 1; i >= 0; i--)
delete mChildren.at(i);
}

for (auto* child : mChildren)
{
child->setParent(nullptr); // prevent each child from inefficiently removing itself from our mChildren vector, since we're about to clear it anyway
delete child;
}
mChildren.clear();
}

Expand Down
2 changes: 2 additions & 0 deletions es-app/src/FileData.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "utils/FileSystemUtil.h"
#include "MetaData.h"
#include <unordered_map>
#include <unordered_set>
#include <memory>
#include <vector>
#include <stack>
Expand Down Expand Up @@ -251,6 +252,7 @@ class FolderData : public FileData

void addChild(FileData* file, bool assignParent = true); // Error if mType != FOLDER
void removeChild(FileData* file); //Error if mType != FOLDER
void bulkRemoveChildren(std::vector<FileData*>& mChildren, const std::unordered_set<FileData*>& filesToRemove); //Error if mType != FOLDER

void createChildrenByFilenameMap(std::unordered_map<std::string, FileData*>& map);

Expand Down

0 comments on commit 61b9844

Please sign in to comment.