From 088405b1ab45e703f66781ee9654edca948d5e6a Mon Sep 17 00:00:00 2001 From: Dan McCarthy Date: Sun, 10 Mar 2024 18:00:50 -0500 Subject: [PATCH] Debugger: Optimize loading results by switching to vector Prior since the results were stored in a hashmap, the .keys() function needed to be used to index at an arbitrary point when loading results into the UI. This caused a big spike in memory usage when the results count is particularly large. Using a vector optimizes this as we don't need to add any memory when indexing in this way. Also unlike before when we used vector, we're also removing elements in place when doing filter searches so we don't need two vectors. --- pcsx2-qt/Debugger/MemorySearchWidget.cpp | 127 ++++++++++------------- pcsx2-qt/Debugger/MemorySearchWidget.h | 4 +- 2 files changed, 59 insertions(+), 72 deletions(-) diff --git a/pcsx2-qt/Debugger/MemorySearchWidget.cpp b/pcsx2-qt/Debugger/MemorySearchWidget.cpp index 75297ac69427c9..53733b8b6a5351 100644 --- a/pcsx2-qt/Debugger/MemorySearchWidget.cpp +++ b/pcsx2-qt/Debugger/MemorySearchWidget.cpp @@ -20,7 +20,6 @@ using SearchComparison = MemorySearchWidget::SearchComparison; using SearchType = MemorySearchWidget::SearchType; using SearchResult = MemorySearchWidget::SearchResult; -using SearchResults = QMap; using namespace QtUtils; @@ -72,7 +71,10 @@ void MemorySearchWidget::contextRemoveSearchResult() const int selectedResultIndex = m_ui.listSearchResults->row(m_ui.listSearchResults->selectedItems().first()); const auto* rowToRemove = m_ui.listSearchResults->takeItem(selectedResultIndex); u32 address = rowToRemove->data(Qt::UserRole).toUInt(); - m_searchResultsMap.remove(address); + if (m_searchResults.size() > static_cast(selectedResultIndex) && m_searchResults.at(selectedResultIndex).getAddress() == address) + { + m_searchResults.erase(m_searchResults.begin() + selectedResultIndex); + } delete rowToRemove; } @@ -224,7 +226,7 @@ static bool memoryValueComparator(SearchComparison searchComparison, T searchVal // Handles the comparison of the read value against either the search value, or if existing searchResults are available, the value at the same address in the searchResultsMap template -bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress, SearchResults& searchResults, T searchValue, T readValue) +bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress, const SearchResult* priorResult, T searchValue, T readValue) { const bool isNotOperator = searchComparison == SearchComparison::NotEquals || searchComparison == SearchComparison::NotChanged; switch (searchComparison) @@ -241,27 +243,27 @@ bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress } case SearchComparison::Increased: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); return memoryValueComparator(SearchComparison::GreaterThan, priorValue, readValue); break; } case SearchComparison::IncreasedBy: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); const T expectedIncrease = searchValue + priorValue; return memoryValueComparator(SearchComparison::Equals, readValue, expectedIncrease); break; } case SearchComparison::Decreased: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); return memoryValueComparator(SearchComparison::LessThan, priorValue, readValue); break; } case SearchComparison::DecreasedBy: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); const T expectedDecrease = priorValue - searchValue; return memoryValueComparator(SearchComparison::Equals, readValue, expectedDecrease); break; @@ -269,13 +271,13 @@ bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress case SearchComparison::Changed: case SearchComparison::NotChanged: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); return memoryValueComparator(isNotOperator ? SearchComparison::Equals : SearchComparison::NotEquals, priorValue, readValue); break; } case SearchComparison::ChangedBy: { - const T priorValue = searchResults.value(searchAddress).getValue(); + const T priorValue = priorResult->getValue(); const T expectedIncrease = searchValue + priorValue; const T expectedDecrease = priorValue - searchValue; return memoryValueComparator(SearchComparison::Equals, readValue, expectedIncrease) || memoryValueComparator(SearchComparison::Equals, readValue, expectedDecrease); @@ -287,9 +289,8 @@ bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress } template -void searchWorker(DebugInterface* cpu, SearchResults& searchResults, SearchType searchType, SearchComparison searchComparison, u32 start, u32 end, T searchValue) +void searchWorker(DebugInterface* cpu, std::vector& searchResults, SearchType searchType, SearchComparison searchComparison, u32 start, u32 end, T searchValue) { - SearchResults newSearchResults; const bool isSearchingRange = searchResults.size() <= 0; if (isSearchingRange) { @@ -299,34 +300,28 @@ void searchWorker(DebugInterface* cpu, SearchResults& searchResults, SearchType continue; T readValue = readValueAtAddress(cpu, addr); - if (handleSearchComparison(searchComparison, addr, searchResults, searchValue, readValue)) + if (handleSearchComparison(searchComparison, addr, nullptr, searchValue, readValue)) { - searchResults.insert(addr, MemorySearchWidget::SearchResult(addr, QVariant::fromValue(readValue), searchType)); + searchResults.push_back(MemorySearchWidget::SearchResult(addr, QVariant::fromValue(readValue), searchType)); } } } else { - for (auto it = searchResults.begin(); it != searchResults.end();) - { - SearchResult searchResult = it.value(); + auto removeIt = std::remove_if(searchResults.begin(), searchResults.end(), [cpu, searchType, searchComparison, searchValue](SearchResult& searchResult) -> bool { const u32 addr = searchResult.getAddress(); if (!cpu->isValidAddress(addr)) - { - it++; - continue; - } - T readValue = readValueAtAddress(cpu, addr); - if (handleSearchComparison(searchComparison, addr, searchResults, searchValue, readValue)) - { - searchResults[addr] = MemorySearchWidget::SearchResult(addr, QVariant::fromValue(readValue), searchType); - it++; - } - else - { - it = searchResults.erase(it); - } - } + return true; + + auto readValue = readValueAtAddress(cpu, addr); + + bool doesMatch = handleSearchComparison(searchComparison, addr, &searchResult, searchValue, readValue); + if (!doesMatch) + searchResult = MemorySearchWidget::SearchResult(addr, QVariant::fromValue(readValue), searchType); + + return !doesMatch; + }); + searchResults.erase(removeIt, searchResults.end()); } } @@ -360,7 +355,7 @@ static bool compareByteArrayAtAddress(DebugInterface* cpu, SearchComparison sear return !isNotOperator; } -bool handleArraySearchComparison(DebugInterface* cpu, SearchComparison searchComparison, u32 searchAddress, SearchResults searchResults, QByteArray searchValue) +bool handleArraySearchComparison(DebugInterface* cpu, SearchComparison searchComparison, u32 searchAddress, SearchResult* priorResult, QByteArray searchValue) { const bool isNotOperator = searchComparison == SearchComparison::NotEquals || searchComparison == SearchComparison::NotChanged; switch (searchComparison) @@ -374,7 +369,7 @@ bool handleArraySearchComparison(DebugInterface* cpu, SearchComparison searchCom case SearchComparison::Changed: case SearchComparison::NotChanged: { - QByteArray priorValue = searchResults.value(searchAddress).getArrayValue(); + QByteArray priorValue = priorResult->getArrayValue(); return compareByteArrayAtAddress(cpu, isNotOperator ? SearchComparison::Equals : SearchComparison::NotEquals, searchAddress, priorValue); break; } @@ -398,7 +393,7 @@ static QByteArray readArrayAtAddress(DebugInterface* cpu, u32 address, u32 lengt return readArray; } -static void searchWorkerByteArray(DebugInterface* cpu, SearchType searchType, SearchComparison searchComparison, SearchResults& searchResults, u32 start, u32 end, QByteArray searchValue) +static void searchWorkerByteArray(DebugInterface* cpu, SearchType searchType, SearchComparison searchComparison, std::vector& searchResults, u32 start, u32 end, QByteArray searchValue) { const bool isSearchingRange = searchResults.size() <= 0; if (isSearchingRange) @@ -407,26 +402,22 @@ static void searchWorkerByteArray(DebugInterface* cpu, SearchType searchType, Se { if (!cpu->isValidAddress(addr)) continue; - if (handleArraySearchComparison(cpu, searchComparison, addr, searchResults, searchValue)) + if (handleArraySearchComparison(cpu, searchComparison, addr, nullptr, searchValue)) { - searchResults.insert(addr, MemorySearchWidget::SearchResult(addr, searchValue, searchType)); + searchResults.push_back(MemorySearchWidget::SearchResult(addr, searchValue, searchType)); addr += searchValue.length() - 1; } } } else { - for (auto it = searchResults.begin(); it != searchResults.end();) - { - SearchResult searchResult = it.value(); + auto removeIt = std::remove_if(searchResults.begin(), searchResults.end(), [ searchComparison, searchType, searchValue, cpu ](SearchResult& searchResult) -> bool { const u32 addr = searchResult.getAddress(); if (!cpu->isValidAddress(addr)) - { - it++; - continue; - } - - if (handleArraySearchComparison(cpu, searchComparison, addr, searchResults, searchValue)) + return true; + + bool doesMatch = handleArraySearchComparison(cpu, searchComparison, addr, &searchResult, searchValue); + if (doesMatch) { QByteArray matchValue; if (searchComparison == SearchComparison::Equals) @@ -435,18 +426,15 @@ static void searchWorkerByteArray(DebugInterface* cpu, SearchType searchType, Se matchValue = searchResult.getArrayValue(); else matchValue = readArrayAtAddress(cpu, addr, searchValue.length() - 1); - searchResults[addr] = MemorySearchWidget::SearchResult(addr, matchValue, searchType); - it++; + searchResult = MemorySearchWidget::SearchResult(addr, matchValue, searchType); } - else - { - it = searchResults.erase(it); - } - } + return !doesMatch; + }); + searchResults.erase(removeIt, searchResults.end()); } } -SearchResults startWorker(DebugInterface* cpu, const SearchType type, const SearchComparison comparison, SearchResults searchResults, u32 start, u32 end, QString value, int base) +std::vector startWorker(DebugInterface* cpu, const SearchType type, const SearchComparison comparison, std::vector searchResults, u32 start, u32 end, QString value, int base) { const bool isSigned = value.startsWith("-"); switch (type) @@ -568,16 +556,16 @@ void MemorySearchWidget::onSearchButtonClicked() return; } - QFutureWatcher* workerWatcher = new QFutureWatcher(); + QFutureWatcher>* workerWatcher = new QFutureWatcher>(); auto onSearchFinished = [this, workerWatcher] { m_ui.btnSearch->setDisabled(false); m_ui.listSearchResults->clear(); const auto& results = workerWatcher->future().result(); - m_searchResultsMap = std::move(results); + m_searchResults = std::move(results); loadSearchResults(); - m_ui.resultsCountLabel->setText(QString(tr("%0 results found")).arg(results.size())); + m_ui.resultsCountLabel->setText(QString(tr("%0 results found")).arg(m_searchResults.size())); m_ui.btnFilterSearch->setDisabled(m_ui.listSearchResults->count() == 0); updateSearchComparisonSelections(); delete workerWatcher; @@ -587,20 +575,20 @@ void MemorySearchWidget::onSearchButtonClicked() m_ui.btnSearch->setDisabled(true); if (!isFilterSearch) { - m_searchResultsMap.clear(); + m_searchResults.clear(); } - QFuture workerFuture = QtConcurrent::run(startWorker, m_cpu, searchType, searchComparison, std::move(m_searchResultsMap), searchStart, searchEnd, searchValue, searchHex ? 16 : 10); + QFuture> workerFuture = QtConcurrent::run(startWorker, m_cpu, searchType, searchComparison, std::move(m_searchResults), searchStart, searchEnd, searchValue, searchHex ? 16 : 10); workerWatcher->setFuture(workerFuture); - connect(workerWatcher, &QFutureWatcher::finished, onSearchFinished); - m_searchResultsMap.clear(); + connect(workerWatcher, &QFutureWatcher>::finished, onSearchFinished); + m_searchResults.clear(); m_ui.resultsCountLabel->setText(tr("Searching...")); m_ui.resultsCountLabel->setVisible(true); } void MemorySearchWidget::onSearchResultsListScroll(u32 value) { - const bool hasResultsToLoad = static_cast(m_ui.listSearchResults->count()) < m_searchResultsMap.size(); + const bool hasResultsToLoad = static_cast(m_ui.listSearchResults->count()) < m_searchResults.size(); const bool scrolledSufficiently = value > (m_ui.listSearchResults->verticalScrollBar()->maximum() * 0.95); if (!m_resultsLoadTimer.isActive() && hasResultsToLoad && scrolledSufficiently) { @@ -612,7 +600,7 @@ void MemorySearchWidget::onSearchResultsListScroll(u32 value) void MemorySearchWidget::loadSearchResults() { const u32 numLoaded = m_ui.listSearchResults->count(); - const u32 amountLeftToLoad = m_searchResultsMap.size() - numLoaded; + const u32 amountLeftToLoad = m_searchResults.size() - numLoaded; if (amountLeftToLoad < 1) return; @@ -620,10 +608,9 @@ void MemorySearchWidget::loadSearchResults() const u32 maxLoadAmount = isFirstLoad ? m_initialResultsLoadLimit : m_numResultsAddedPerLoad; const u32 numToLoad = amountLeftToLoad > maxLoadAmount ? maxLoadAmount : amountLeftToLoad; - const auto addresses = m_searchResultsMap.keys(); for (u32 i = 0; i < numToLoad; i++) { - const u32 address = addresses.at(numLoaded + i); + const u32 address = m_searchResults.at(numLoaded + i).getAddress(); QListWidgetItem* item = new QListWidgetItem(QtUtils::FilledQStringFromValue(address, 16)); item->setData(Qt::UserRole, address); m_ui.listSearchResults->addItem(item); @@ -649,9 +636,9 @@ void MemorySearchWidget::onSearchTypeChanged(int newIndex) m_ui.chkSearchHex->setEnabled(false); // Clear existing search results when the comparison type changes - if (m_searchResultsMap.size() > 0 && (int)(m_searchResultsMap.first().getType()) != newIndex) + if (m_searchResults.size() > 0 && (int)(m_searchResults.front().getType()) != newIndex) { - m_searchResultsMap.clear(); + m_searchResults.clear(); m_ui.btnSearch->setDisabled(false); m_ui.btnFilterSearch->setDisabled(true); } @@ -663,7 +650,7 @@ void MemorySearchWidget::updateSearchComparisonSelections() const QString selectedComparisonLabel = m_ui.cmbSearchComparison->currentText(); const SearchComparison selectedComparison = m_searchComparisonLabelMap.labelToEnum(selectedComparisonLabel); - const std::vector comparisons = getValidSearchComparisonsForState(getCurrentSearchType(), m_searchResultsMap); + const std::vector comparisons = getValidSearchComparisonsForState(getCurrentSearchType(), m_searchResults); m_ui.cmbSearchComparison->clear(); for (const SearchComparison comparison : comparisons) { @@ -677,14 +664,14 @@ void MemorySearchWidget::updateSearchComparisonSelections() m_ui.cmbSearchComparison->setCurrentText(selectedComparisonLabel); } -std::vector MemorySearchWidget::getValidSearchComparisonsForState(SearchType type, SearchResults existingResults) +std::vector MemorySearchWidget::getValidSearchComparisonsForState(SearchType type, std::vector& existingResults) { const bool hasResults = existingResults.size() > 0; std::vector comparisons = { SearchComparison::Equals }; if (type == SearchType::ArrayType || type == SearchType::StringType) { - if (hasResults && existingResults.first().isArrayValue()) + if (hasResults && existingResults.front().isArrayValue()) { comparisons.push_back(SearchComparison::NotEquals); comparisons.push_back(SearchComparison::Changed); @@ -698,7 +685,7 @@ std::vector MemorySearchWidget::getValidSearchComparisonsForSt comparisons.push_back(SearchComparison::LessThan); comparisons.push_back(SearchComparison::LessThanOrEqual); - if (hasResults && existingResults.first().getType() == type) + if (hasResults && existingResults.front().getType() == type) { comparisons.push_back(SearchComparison::Increased); comparisons.push_back(SearchComparison::IncreasedBy); diff --git a/pcsx2-qt/Debugger/MemorySearchWidget.h b/pcsx2-qt/Debugger/MemorySearchWidget.h index 28ff0a372cfaf7..37412fce70a9cc 100644 --- a/pcsx2-qt/Debugger/MemorySearchWidget.h +++ b/pcsx2-qt/Debugger/MemorySearchWidget.h @@ -133,7 +133,7 @@ public slots: void switchToMemoryViewTab(); private: - QMap m_searchResultsMap; + std::vector m_searchResults; SearchComparisonLabelMap m_searchComparisonLabelMap; Ui::MemorySearchWidget m_ui; DebugInterface* m_cpu; @@ -143,7 +143,7 @@ public slots: u32 m_numResultsAddedPerLoad = 10000; void updateSearchComparisonSelections(); - std::vector getValidSearchComparisonsForState(SearchType type, QMap existingResults); + std::vector getValidSearchComparisonsForState(SearchType type, std::vector &existingResults); SearchType getCurrentSearchType(); SearchComparison getCurrentSearchComparison(); };