From 08188830a3cf67924980b4183e2ca7a4b5e12c3d Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Tue, 14 Jan 2020 16:52:33 -0500 Subject: removed LEAK_TRACE stuff refactored DirectoryEntry for lookup maps filetreemodel: made ensureLoaded() a no-op while refreshing, faster processing for removing rows --- src/filetreeitem.cpp | 13 +++ src/filetreeitem.h | 1 + src/filetreemodel.cpp | 66 ++++++++++--- src/filetreemodel.h | 1 + src/shared/directoryentry.cpp | 219 ++++++++++++++++++------------------------ src/shared/directoryentry.h | 20 ++-- 6 files changed, 174 insertions(+), 146 deletions(-) diff --git a/src/filetreeitem.cpp b/src/filetreeitem.cpp index 39cdd9c4..33a70798 100644 --- a/src/filetreeitem.cpp +++ b/src/filetreeitem.cpp @@ -48,6 +48,19 @@ void FileTreeItem::remove(std::size_t i) m_children.erase(m_children.begin() + i); } +void FileTreeItem::remove(std::size_t from, std::size_t n) +{ + if ((from + n) > m_children.size()) { + log::error("{}: can't remove children from {} n={}", debugName(), from, n); + return; + } + + auto begin = m_children.begin() + from; + auto end = begin + n; + + m_children.erase(begin, end); +} + QString FileTreeItem::virtualPath() const { QString s = "Data\\"; diff --git a/src/filetreeitem.h b/src/filetreeitem.h index fb9eccce..70757ca2 100644 --- a/src/filetreeitem.h +++ b/src/filetreeitem.h @@ -34,6 +34,7 @@ public: void insert(std::unique_ptr child, std::size_t at); void remove(std::size_t i); + void remove(std::size_t from, std::size_t n); void clear() { diff --git a/src/filetreemodel.cpp b/src/filetreemodel.cpp index ed62b8ae..91dd4b90 100644 --- a/src/filetreemodel.cpp +++ b/src/filetreemodel.cpp @@ -20,7 +20,7 @@ void trace(F&&) FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) : QAbstractItemModel(parent), m_core(core), m_root(nullptr, 0, L"", L"", FileTreeItem::Directory, L"", L""), - m_flags(NoFlags) + m_flags(NoFlags), m_isRefreshing(false) { m_root.setExpanded(true); @@ -39,6 +39,9 @@ FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) : void FileTreeModel::refresh() { + m_isRefreshing = true; + Guard g([&]{ m_isRefreshing = false; }); + if (m_root.hasChildren()) { TimeThis tt("FileTreeModel::update()"); update(m_root, *m_core.directoryStructure(), L""); @@ -65,6 +68,10 @@ bool FileTreeModel::showArchives() const void FileTreeModel::ensureLoaded(FileTreeItem* item) const { + if (m_isRefreshing) { + return; + } + if (!item) { log::error("ensureLoaded(): item is null"); return; @@ -247,7 +254,7 @@ void FileTreeModel::updateDirectories( }); int row = 0; - std::vector remove; + std::list remove; std::unordered_set seen; for (auto&& item : parentItem.children()) { @@ -329,26 +336,59 @@ void FileTreeModel::updateDirectories( parentItem.debugName()); }); - for (auto* toRemove : remove) { - const auto& cs = parentItem.children(); - for (std::size_t i=0; i(i), 0); + if (!parentIndex.isValid()) { + parentIndex = parent(indexFromItem( + toRemove, static_cast(i), 0)); + } - const auto parentIndex = parent(itemIndex); - const int first = static_cast(i); - const int last = static_cast(i); + if (first == -1) { + first = i; + last = i; + } else if (i == (last + 1)) { + last = i; + } else { + beginRemoveRows(parentIndex, first, last); - beginRemoveRows(parentIndex, first, last); - parentItem.remove(i); - endRemoveRows(); + parentItem.remove( + static_cast(first), + static_cast(last - first + 1)); + + endRemoveRows(); + + first = i; + last = i; + } + remove.erase(itor); break; } } } + + if (first != -1) { + beginRemoveRows(parentIndex, first, last); + + parentItem.remove( + static_cast(first), + static_cast(last - first + 1)); + + endRemoveRows(); + } } diff --git a/src/filetreemodel.h b/src/filetreemodel.h index 8a1738b3..c363266a 100644 --- a/src/filetreemodel.h +++ b/src/filetreemodel.h @@ -57,6 +57,7 @@ private: mutable IconFetcher m_iconFetcher; mutable std::vector m_iconPending; mutable QTimer m_iconPendingTimer; + bool m_isRefreshing; bool showConflicts() const { diff --git a/src/shared/directoryentry.cpp b/src/shared/directoryentry.cpp index c44be9a1..819075ae 100644 --- a/src/shared/directoryentry.cpp +++ b/src/shared/directoryentry.cpp @@ -19,7 +19,6 @@ along with Mod Organizer. If not, see . #include "directoryentry.h" #include "windows_error.h" -#include "leaktrace.h" #include "error_report.h" #include #include @@ -32,7 +31,6 @@ along with Mod Organizer. If not, see . #include #include #include -#include namespace MOShared { @@ -80,12 +78,6 @@ public: OriginConnection() : m_NextID(0) { - LEAK_TRACE; - } - - ~OriginConnection() - { - LEAK_UNTRACE; } FilesOrigin& createOrigin( @@ -166,19 +158,12 @@ FileEntry::FileEntry() : m_Index(UINT_MAX), m_Name(), m_Origin(-1), m_Parent(nullptr), m_LastAccessed(time(nullptr)) { - LEAK_TRACE; } FileEntry::FileEntry(Index index, const std::wstring &name, DirectoryEntry *parent) : m_Index(index), m_Name(name), m_Origin(-1), m_Archive(L"", -1), m_Parent(parent), m_LastAccessed(time(nullptr)) { - LEAK_TRACE; -} - -FileEntry::~FileEntry() -{ - LEAK_UNTRACE; } void FileEntry::addOrigin( @@ -399,7 +384,6 @@ bool FileEntry::recurseParents(std::wstring &path, const DirectoryEntry *parent) FilesOrigin::FilesOrigin() : m_ID(0), m_Disabled(false), m_Name(), m_Path(), m_Priority(0) { - LEAK_TRACE; } FilesOrigin::FilesOrigin(const FilesOrigin &reference) @@ -411,7 +395,6 @@ FilesOrigin::FilesOrigin(const FilesOrigin &reference) , m_FileRegister(reference.m_FileRegister) , m_OriginConnection(reference.m_OriginConnection) { - LEAK_TRACE; } FilesOrigin::FilesOrigin( @@ -422,12 +405,6 @@ FilesOrigin::FilesOrigin( m_Priority(priority), m_FileRegister(fileRegister), m_OriginConnection(originConnection) { - LEAK_TRACE; -} - -FilesOrigin::~FilesOrigin() -{ - LEAK_UNTRACE; } void FilesOrigin::setPriority(int priority) @@ -504,13 +481,6 @@ bool FilesOrigin::containsArchive(std::wstring archiveName) FileRegister::FileRegister(boost::shared_ptr originConnection) : m_OriginConnection(originConnection) { - LEAK_TRACE; -} - -FileRegister::~FileRegister() -{ - LEAK_UNTRACE; - m_Files.clear(); } bool FileRegister::indexValid(FileEntry::Index index) const @@ -650,7 +620,6 @@ DirectoryEntry::DirectoryEntry( { m_FileRegister.reset(new FileRegister(m_OriginConnection)); m_Origins.insert(originID); - LEAK_TRACE; } DirectoryEntry::DirectoryEntry( @@ -660,13 +629,11 @@ DirectoryEntry::DirectoryEntry( m_FileRegister(fileRegister), m_OriginConnection(originConnection), m_Name(name), m_Parent(parent), m_Populated(false), m_TopLevel(false) { - LEAK_TRACE; m_Origins.insert(originID); } DirectoryEntry::~DirectoryEntry() { - LEAK_UNTRACE; clear(); } @@ -680,7 +647,7 @@ void DirectoryEntry::clear() } m_SubDirectories.clear(); - m_SubDirectoriesMap.clear(); + m_SubDirectoriesLookup.clear(); } void DirectoryEntry::addFromOrigin( @@ -735,7 +702,7 @@ void DirectoryEntry::addFromBSA( stream << QObject::tr("invalid bsa file: ").toStdString() << ToString(fileName, false) - << " errorcode " << res << " - " << ::GetLastError(); + << " error code " << res << " - " << ::GetLastError(); throw std::runtime_error(stream.str()); } @@ -806,15 +773,15 @@ std::vector DirectoryEntry::getFiles() const DirectoryEntry *DirectoryEntry::findSubDirectory( const std::wstring &name, bool alreadyLowerCase) const { - SubDirectoriesMap::const_iterator itor; + SubDirectoriesLookup::const_iterator itor; if (alreadyLowerCase) { - itor = m_SubDirectoriesMap.find(name); + itor = m_SubDirectoriesLookup.find(name); } else { - itor = m_SubDirectoriesMap.find(ToLowerCopy(name)); + itor = m_SubDirectoriesLookup.find(ToLowerCopy(name)); } - if (itor == m_SubDirectoriesMap.end()) { + if (itor == m_SubDirectoriesLookup.end()) { return nullptr; } @@ -936,48 +903,7 @@ void DirectoryEntry::insertFile( void DirectoryEntry::removeFile(FileEntry::Index index) { - if (!m_FilesLookup.empty()) { - auto iter = std::find_if( - m_FilesLookup.begin(), m_FilesLookup.end(), - [&index](auto&& pair) { return (pair.second == index); } - ); - - if (iter != m_FilesLookup.end()) { - m_FilesLookup.erase(iter); - } else { - log::error( - QObject::tr("file \"{}\" not in directory for lookup \"{}\"").toStdString(), - m_FileRegister->getFile(index)->getName(), this->getName()); - } - } else { - log::error( - QObject::tr("file \"{}\" not in directory \"{}\" for lookup, directory empty").toStdString(), - m_FileRegister->getFile(index)->getName(), this->getName()); - } - - if (!m_Files.empty()) { - auto iter = std::find_if( - m_Files.begin(), m_Files.end(), - [&index](const std::pair &iter) -> bool { - return iter.second == index; } - ); - - if (iter != m_Files.end()) { - m_Files.erase(iter); - } else { - log::error( - QObject::tr("file \"{}\" not in directory \"{}\"").toStdString(), - m_FileRegister->getFile(index)->getName(), this->getName()); - } - } else { - log::error( - QObject::tr("file \"{}\" not in directory \"{}\", directory empty").toStdString(), - m_FileRegister->getFile(index)->getName(), this->getName()); - } - - if (m_Files.size() != m_FilesLookup.size()) { - DebugBreak(); - } + removeFileFromList(index); } bool DirectoryEntry::removeFile(const std::wstring &filePath, int *origin) @@ -1009,21 +935,7 @@ void DirectoryEntry::removeDir(const std::wstring &path) if (CaseInsensitiveEqual(entry->getName(), path)) { entry->removeDirRecursive(); - - bool found = false; - for (auto iter2=m_SubDirectoriesMap.begin(); iter2!=m_SubDirectoriesMap.end(); ++iter2) { - if (iter2->second == entry) { - m_SubDirectoriesMap.erase(iter2); - found = true; - break; - } - } - - if (!found) { - log::error("entry {} not in sub directories map", entry->getName()); - } - - m_SubDirectories.erase(iter); + removeDirectoryFromList(iter); delete entry; break; } @@ -1058,10 +970,6 @@ bool DirectoryEntry::remove(const std::wstring &fileName, int *origin) b = m_FileRegister->removeFile(iter->second); } - if (m_Files.size() != m_FilesLookup.size()) { - DebugBreak(); - } - return b; } @@ -1085,41 +993,21 @@ FilesOrigin &DirectoryEntry::createOrigin( void DirectoryEntry::removeFiles(const std::set &indices) { - for (auto iter = m_Files.begin(); iter != m_Files.end();) { - if (indices.find(iter->second) != indices.end()) { - iter = m_Files.erase(iter); - } else { - ++iter; - } - } - - for (auto iter = m_FilesLookup.begin(); iter != m_FilesLookup.end();) { - if (indices.find(iter->second) != indices.end()) { - iter = m_FilesLookup.erase(iter); - } else { - ++iter; - } - } + removeFilesFromList(indices); } void DirectoryEntry::insert( const std::wstring &fileName, FilesOrigin &origin, FILETIME fileTime, const std::wstring &archive, int order) { - std::wstring fileNameLower = ToLowerCopy(fileName); - auto iter = m_Files.find(fileNameLower); + auto iter = m_Files.find(ToLowerCopy(fileName)); FileEntry::Ptr file; if (iter != m_Files.end()) { file = m_FileRegister->getFile(iter->second); } else { file = m_FileRegister->createFile(fileName, this); - m_Files.emplace(fileNameLower, file->getIndex()); - m_FilesLookup.emplace(fileNameLower, file->getIndex()); - } - - if (m_Files.size() != m_FilesLookup.size()) { - DebugBreak(); + addFileToList(*file); } file->addOrigin(origin.getID(), fileTime, archive, order); @@ -1198,8 +1086,7 @@ DirectoryEntry *DirectoryEntry::getSubDirectory( auto* entry = new DirectoryEntry( name, this, originID, m_FileRegister, m_OriginConnection); - m_SubDirectories.push_back(entry); - m_SubDirectoriesMap.emplace(ToLowerCopy(name), entry); + addDirectoryToList(entry); return entry; } else { @@ -1244,7 +1131,87 @@ void DirectoryEntry::removeDirRecursive() } m_SubDirectories.clear(); - m_SubDirectoriesMap.clear(); + m_SubDirectoriesLookup.clear(); +} + +void DirectoryEntry::addDirectoryToList(DirectoryEntry* e) +{ + m_SubDirectories.push_back(e); + m_SubDirectoriesLookup.emplace(ToLowerCopy(e->getName()), e); +} + +void DirectoryEntry::removeDirectoryFromList(SubDirectories::iterator itor) +{ + const auto* entry = *itor; + + { + auto itor2 = std::find_if( + m_SubDirectoriesLookup.begin(), m_SubDirectoriesLookup.end(), + [&](auto&& d) { return (d.second == entry); }); + + if (itor2 == m_SubDirectoriesLookup.end()) { + log::error("entry {} not in sub directories map", entry->getName()); + } else { + m_SubDirectoriesLookup.erase(itor2); + } + } + + m_SubDirectories.erase(itor); +} + +void DirectoryEntry::removeFileFromList(FileEntry::Index index) +{ + auto removeFrom = [&](auto& list) { + auto iter = std::find_if( + list.begin(), list.end(), + [&index](auto&& pair) { return (pair.second == index); } + ); + + if (iter == list.end()) { + auto f = m_FileRegister->getFile(index); + + if (f) { + log::error( + "can't remove file '{}', not in directory entry '{}'", + f->getName(), getName()); + } else { + log::error( + "can't remove file with index {}, not in directory entry '{}' and " + "not in register", + index, getName()); + } + } else { + list.erase(iter); + } + }; + + removeFrom(m_FilesLookup); + removeFrom(m_Files); +} + +void DirectoryEntry::removeFilesFromList(const std::set& indices) +{ + for (auto iter = m_Files.begin(); iter != m_Files.end();) { + if (indices.find(iter->second) != indices.end()) { + iter = m_Files.erase(iter); + } else { + ++iter; + } + } + + for (auto iter = m_FilesLookup.begin(); iter != m_FilesLookup.end();) { + if (indices.find(iter->second) != indices.end()) { + iter = m_FilesLookup.erase(iter); + } else { + ++iter; + } + } +} + +void DirectoryEntry::addFileToList(const FileEntry& f) +{ + m_Files.emplace(ToLowerCopy(f.getName()), f.getIndex()); + m_FilesLookup.emplace(ToLowerCopy(f.getName()), f.getIndex()); } } // namespace MOShared diff --git a/src/shared/directoryentry.h b/src/shared/directoryentry.h index 98366493..91c2a140 100644 --- a/src/shared/directoryentry.h +++ b/src/shared/directoryentry.h @@ -67,7 +67,6 @@ public: FileEntry(); FileEntry(Index index, const std::wstring &name, DirectoryEntry *parent); - ~FileEntry(); Index getIndex() const { @@ -160,7 +159,6 @@ class FilesOrigin public: FilesOrigin(); FilesOrigin(const FilesOrigin &reference); - ~FilesOrigin(); // sets priority for this origin, but it will overwrite the existing mapping // for this priority, the previous origin will no longer be referenced @@ -226,7 +224,6 @@ class FileRegister { public: FileRegister(boost::shared_ptr originConnection); - ~FileRegister(); bool indexValid(FileEntry::Index index) const; @@ -351,7 +348,8 @@ public: std::vector::const_iterator &begin, std::vector::const_iterator &end) const { - begin = m_SubDirectories.begin(); end = m_SubDirectories.end(); + begin = m_SubDirectories.begin(); + end = m_SubDirectories.end(); } template @@ -442,7 +440,8 @@ public: private: using FilesMap = std::map; using FilesLookup = std::unordered_map; - using SubDirectoriesMap = std::unordered_map; + using SubDirectories = std::vector; + using SubDirectoriesLookup = std::unordered_map; boost::shared_ptr m_FileRegister; boost::shared_ptr m_OriginConnection; @@ -450,8 +449,8 @@ private: std::wstring m_Name; FilesMap m_Files; FilesLookup m_FilesLookup; - std::vector m_SubDirectories; - SubDirectoriesMap m_SubDirectoriesMap; + SubDirectories m_SubDirectories; + SubDirectoriesLookup m_SubDirectoriesLookup; DirectoryEntry *m_Parent; std::set m_Origins; @@ -479,6 +478,13 @@ private: const std::wstring &path, bool create, int originID = -1); void removeDirRecursive(); + + void addDirectoryToList(DirectoryEntry* e); + void removeDirectoryFromList(SubDirectories::iterator itor); + + void addFileToList(const FileEntry& f); + void removeFileFromList(FileEntry::Index index); + void removeFilesFromList(const std::set& indices); }; } // namespace MOShared -- cgit v1.3.1