[DSC] Performance improvements and clean-up in SymbolTableView

Avoid copying the vector of symbols where possible. When no filter
string is applied we no longer copy the entire vector symbols by having
the symbols used for display be indirected via a pointer. It points
either to the unfiltered symbols or the filtered symbols.

Some unncessary copies have been removed by using `std::move` where
appropriate.

Filtering has been updated to avoid `std::vector::reserve` /
`std::vector::shrink_to_fit` in favor of letting the filtered vector
control its own growth and reuse its buffer. This avoids allocating a
~100MB buffer every time we update the filter only to later reallocate
and move the contents into a smaller buffer. Instead the buffer grows
via `std::vector`'s usual growth algorithm and it is preserved across
filter calls to avoid unnecessarily reallocating it, and only shrink the
buffer if it has shrunk significantly vs previous iterations.

2 files changed, 42 insertions, 31 deletions

diff --git a/view/sharedcache/ui/symboltable.cpp b/view/sharedcache/ui/symboltable.cpp
index b0247f2b..e4c5dcd8 100644
--- a/view/sharedcache/ui/symboltable.cpp
+++ b/view/sharedcache/ui/symboltable.cpp
@@ -11,8 +11,9 @@ using namespace BinaryNinja;
 using namespace SharedCacheAPI;
 
 
-SymbolTableModel::SymbolTableModel(SymbolTableView* parent)
-	: QAbstractTableModel(parent), m_parent(parent) {
+SymbolTableModel::SymbolTableModel(SymbolTableView* parent) :
+	QAbstractTableModel(parent), m_parent(parent), m_displaySymbols(&m_symbols)
+{
 	// TODO: Need to implement updating this font if it is changed by the user
 	m_font = getMonospaceFont(parent);
 }
@@ -20,7 +21,7 @@ SymbolTableModel::SymbolTableModel(SymbolTableView* parent)
 
 int SymbolTableModel::rowCount(const QModelIndex& parent) const {
 	Q_UNUSED(parent);
-	return static_cast<int>(m_modelSymbols.size());
+	return static_cast<int>(m_displaySymbols->size());
 }
 
 
@@ -111,30 +112,28 @@ void SymbolTableModel::sort(int column, Qt::SortOrder order)
 
 	if (order == Qt::DescendingOrder)
 	{
-		std::sort(m_modelSymbols.begin(), m_modelSymbols.end(),
-				  [&comparator](const CacheSymbol& a, const CacheSymbol& b) {
-					  return comparator(b, a);
-				  });
+		std::sort(m_displaySymbols->begin(), m_displaySymbols->end(),
+			[&comparator](const CacheSymbol& a, const CacheSymbol& b) { return comparator(b, a); });
 	}
 	else
 	{
-		std::sort(m_modelSymbols.begin(), m_modelSymbols.end(), comparator);
+		std::sort(m_displaySymbols->begin(), m_displaySymbols->end(), comparator);
 	}
 
 	endResetModel();
 }
 
 
-void SymbolTableModel::updateSymbols(std::vector<CacheSymbol>&& symbols)
+void SymbolTableModel::updateSymbols(std::vector<CacheSymbol> symbols)
 {
-	m_preparedSymbols = symbols;
+	m_symbols = std::move(symbols);
 	setFilter(m_filter);
 }
 
 
 const CacheSymbol& SymbolTableModel::symbolAt(int row) const
 {
-	return m_modelSymbols.at(row);
+	return m_displaySymbols->at(row);
 }
 
 
@@ -142,30 +141,40 @@ void SymbolTableModel::setFilter(std::string text)
 {
 	beginResetModel();
 
-	m_filter = text;
-	m_modelSymbols = {};
+	m_filter = std::move(text);
 
 	// Skip filtering if no filter applied.
-	if (!m_filter.empty())
+	if (m_filter.empty())
 	{
-		m_modelSymbols.reserve(m_preparedSymbols.size());
-		for (const auto& symbol : m_preparedSymbols)
-			if (((std::string_view)symbol.name).find(m_filter) != std::string::npos)
-				m_modelSymbols.push_back(symbol);
-		m_modelSymbols.shrink_to_fit();
+		m_filteredSymbols = {};
+		m_displaySymbols = &m_symbols;
 	}
 	else
 	{
-		m_modelSymbols = m_preparedSymbols;
+		// Clear the filtered symbols while preserving the capacity
+		m_filteredSymbols.clear();
+
+		for (const auto& symbol : m_symbols)
+		{
+			if (symbol.name.find(m_filter) != std::string::npos)
+				m_filteredSymbols.push_back(symbol);
+		}
+
+		// If the filtered vector is using less than 25% of its capacity,
+		// shrink it to reduce memory usage.
+		if (m_filteredSymbols.size() < m_filteredSymbols.capacity() / 4)
+			m_filteredSymbols.shrink_to_fit();
+
+		m_displaySymbols = &m_filteredSymbols;
 	}
 
 	endResetModel();
 }
 
 
-SymbolTableView::SymbolTableView(QWidget* parent)
-	: QTableView(parent), m_model(new SymbolTableModel(this)) {
-
+SymbolTableView::SymbolTableView(QWidget* parent) :
+	QTableView(parent), m_model(new SymbolTableModel(this))
+{
 	// Set up the filter model
 	setModel(m_model);
 
@@ -181,9 +190,7 @@ SymbolTableView::SymbolTableView(QWidget* parent)
 	setSortingEnabled(true);
 }
 
-SymbolTableView::~SymbolTableView() {
-	delete m_model;
-}
+SymbolTableView::~SymbolTableView() = default;
 
 void SymbolTableView::populateSymbols(BinaryView &view)
 {
diff --git a/view/sharedcache/ui/symboltable.h b/view/sharedcache/ui/symboltable.h
index 51ba88f0..9c7a1eab 100644
--- a/view/sharedcache/ui/symboltable.h
+++ b/view/sharedcache/ui/symboltable.h
@@ -19,9 +19,12 @@ Q_OBJECT
 	SymbolTableView* m_parent;
 	QFont m_font;
 	std::string m_filter;
-	std::vector<SharedCacheAPI::CacheSymbol> m_preparedSymbols{};
-	// These are the symbols we actually use
-	std::vector<SharedCacheAPI::CacheSymbol> m_modelSymbols{};
+
+	std::vector<SharedCacheAPI::CacheSymbol> m_symbols;
+	std::vector<SharedCacheAPI::CacheSymbol> m_filteredSymbols;
+
+	// A pointer to either m_symbols or m_filteredSymbols, depending on whether a filter is applied.
+	std::vector<SharedCacheAPI::CacheSymbol> *m_displaySymbols = nullptr;
 
 public:
 	explicit SymbolTableModel(SymbolTableView* parent);
@@ -31,10 +34,11 @@ public:
 	QVariant data(const QModelIndex& index, int role) const override;
 	QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
 	void sort(int column, Qt::SortOrder order) override;
-	void updateSymbols(std::vector<SharedCacheAPI::CacheSymbol>&& symbols);
+
+	void updateSymbols(std::vector<SharedCacheAPI::CacheSymbol> symbols);
 	void setFilter(std::string text);
-	const SharedCacheAPI::CacheSymbol& symbolAt(int row) const;
 
+	const SharedCacheAPI::CacheSymbol& symbolAt(int row) const;
 };