removed LEAK_TRACE stuff

refactored DirectoryEntry for lookup maps
filetreemodel: made ensureLoaded() a no-op while refreshing, faster processing for removing rows

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<FileTreeItem> 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"<root>"),
-  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<FileTreeItem*> remove;
+  std::list<FileTreeItem*> remove;
   std::unordered_set<std::wstring_view> 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<cs.size(); ++i) {
+    QModelIndex parentIndex;
+    int first = -1;
+    int last = -1;
+
+    const auto& cs = parentItem.children();
+    for (std::size_t i=0; i<cs.size(); ++i) {
+      if (remove.empty()) {
+        break;
+      }
+
+      for (auto itor=remove.begin(); itor!=remove.end(); ++itor) {
+        auto* toRemove = *itor;
+
         if (cs[i].get() == toRemove) {
-          const auto itemIndex = indexFromItem(
-            toRemove, static_cast<int>(i), 0);
+          if (!parentIndex.isValid()) {
+            parentIndex = parent(indexFromItem(
+              toRemove, static_cast<int>(i), 0));
+          }
 
-          const auto parentIndex = parent(itemIndex);
-          const int first = static_cast<int>(i);
-          const int last = static_cast<int>(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<std::size_t>(first),
+              static_cast<std::size_t>(last - first + 1));
+
+            endRemoveRows();
+
+            first = i;
+            last = i;
+          }
 
+          remove.erase(itor);
           break;
         }
       }
     }
+
+    if (first != -1) {
+      beginRemoveRows(parentIndex, first, last);
+
+      parentItem.remove(
+        static_cast<std::size_t>(first),
+        static_cast<std::size_t>(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<QModelIndex> 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 <http://www.gnu.org/licenses/>.
 

 #include "directoryentry.h"

 #include "windows_error.h"

-#include "leaktrace.h"

 #include "error_report.h"

 #include <log.h>

 #include <bsatk.h>

@@ -32,7 +31,6 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include <algorithm>

 #include <map>

 #include <atomic>

-#include <QObject>

 

 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> 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<FileEntry::Ptr> 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<std::wstring, FileEntry::Index> &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<FileEntry::Index> &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<FileEntry::Index>& 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> originConnection);

-  ~FileRegister();

 

   bool indexValid(FileEntry::Index index) const;

 

@@ -351,7 +348,8 @@ public:
     std::vector<DirectoryEntry*>::const_iterator &begin,

     std::vector<DirectoryEntry*>::const_iterator &end) const

   {

-    begin = m_SubDirectories.begin(); end = m_SubDirectories.end();

+    begin = m_SubDirectories.begin();

+    end = m_SubDirectories.end();

   }

 

   template <class F>

@@ -442,7 +440,8 @@ public:
 private:

   using FilesMap = std::map<std::wstring, FileEntry::Index>;

   using FilesLookup = std::unordered_map<FileKey, FileEntry::Index>;

-  using SubDirectoriesMap = std::unordered_map<std::wstring, DirectoryEntry*>;

+  using SubDirectories = std::vector<DirectoryEntry*>;

+  using SubDirectoriesLookup = std::unordered_map<std::wstring, DirectoryEntry*>;

 

   boost::shared_ptr<FileRegister> m_FileRegister;

   boost::shared_ptr<OriginConnection> m_OriginConnection;

@@ -450,8 +449,8 @@ private:
   std::wstring m_Name;

   FilesMap m_Files;

   FilesLookup m_FilesLookup;

-  std::vector<DirectoryEntry*> m_SubDirectories;

-  SubDirectoriesMap m_SubDirectoriesMap;

+  SubDirectories m_SubDirectories;

+  SubDirectoriesLookup m_SubDirectoriesLookup;

 

   DirectoryEntry *m_Parent;

   std::set<int> 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<FileEntry::Index>& indices);

 };

 

 } // namespace MOShared