Skip to content

Commit

Permalink
Debugger: Optimize loading results by switching to vector
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Daniel-McCarthy
Daniel-McCarthy committed Mar 10, 2024
1 parent 175df86 commit 088405b
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 72 deletions.
127 changes: 57 additions & 70 deletions pcsx2-qt/Debugger/MemorySearchWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
using SearchComparison = MemorySearchWidget::SearchComparison;
using SearchType = MemorySearchWidget::SearchType;
using SearchResult = MemorySearchWidget::SearchResult;
using SearchResults = QMap<u32, MemorySearchWidget::SearchResult>;

using namespace QtUtils;

Expand Down Expand Up @@ -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<size_t>(selectedResultIndex) && m_searchResults.at(selectedResultIndex).getAddress() == address)
{
m_searchResults.erase(m_searchResults.begin() + selectedResultIndex);
}
delete rowToRemove;
}

Expand Down Expand Up @@ -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 <typename T>
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)
Expand All @@ -241,41 +243,41 @@ bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress
}
case SearchComparison::Increased:
{
const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
return memoryValueComparator(SearchComparison::GreaterThan, priorValue, readValue);
break;
}
case SearchComparison::IncreasedBy:
{

const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
const T expectedIncrease = searchValue + priorValue;
return memoryValueComparator(SearchComparison::Equals, readValue, expectedIncrease);
break;
}
case SearchComparison::Decreased:
{
const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
return memoryValueComparator(SearchComparison::LessThan, priorValue, readValue);
break;
}
case SearchComparison::DecreasedBy:
{
const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
const T expectedDecrease = priorValue - searchValue;
return memoryValueComparator(SearchComparison::Equals, readValue, expectedDecrease);
break;
}
case SearchComparison::Changed:
case SearchComparison::NotChanged:
{
const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
return memoryValueComparator(isNotOperator ? SearchComparison::Equals : SearchComparison::NotEquals, priorValue, readValue);
break;
}
case SearchComparison::ChangedBy:
{
const T priorValue = searchResults.value(searchAddress).getValue<T>();
const T priorValue = priorResult->getValue<T>();
const T expectedIncrease = searchValue + priorValue;
const T expectedDecrease = priorValue - searchValue;
return memoryValueComparator(SearchComparison::Equals, readValue, expectedIncrease) || memoryValueComparator(SearchComparison::Equals, readValue, expectedDecrease);
Expand All @@ -287,9 +289,8 @@ bool handleSearchComparison(SearchComparison searchComparison, u32 searchAddress
}

template <typename T>
void searchWorker(DebugInterface* cpu, SearchResults& searchResults, SearchType searchType, SearchComparison searchComparison, u32 start, u32 end, T searchValue)
void searchWorker(DebugInterface* cpu, std::vector<SearchResult>& searchResults, SearchType searchType, SearchComparison searchComparison, u32 start, u32 end, T searchValue)
{
SearchResults newSearchResults;
const bool isSearchingRange = searchResults.size() <= 0;
if (isSearchingRange)
{
Expand All @@ -299,34 +300,28 @@ void searchWorker(DebugInterface* cpu, SearchResults& searchResults, SearchType
continue;

T readValue = readValueAtAddress<T>(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<T>(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<T>(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());
}
}

Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand All @@ -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<SearchResult>& searchResults, u32 start, u32 end, QByteArray searchValue)
{
const bool isSearchingRange = searchResults.size() <= 0;
if (isSearchingRange)
Expand All @@ -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)
Expand All @@ -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<SearchResult> startWorker(DebugInterface* cpu, const SearchType type, const SearchComparison comparison, std::vector<SearchResult> searchResults, u32 start, u32 end, QString value, int base)
{
const bool isSigned = value.startsWith("-");
switch (type)
Expand Down Expand Up @@ -568,16 +556,16 @@ void MemorySearchWidget::onSearchButtonClicked()
return;
}

QFutureWatcher<SearchResults>* workerWatcher = new QFutureWatcher<SearchResults>();
QFutureWatcher<std::vector<SearchResult>>* workerWatcher = new QFutureWatcher<std::vector<SearchResult>>();
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;
Expand All @@ -587,20 +575,20 @@ void MemorySearchWidget::onSearchButtonClicked()
m_ui.btnSearch->setDisabled(true);
if (!isFilterSearch)
{
m_searchResultsMap.clear();
m_searchResults.clear();
}

QFuture<SearchResults> workerFuture = QtConcurrent::run(startWorker, m_cpu, searchType, searchComparison, std::move(m_searchResultsMap), searchStart, searchEnd, searchValue, searchHex ? 16 : 10);
QFuture<std::vector<SearchResult>> workerFuture = QtConcurrent::run(startWorker, m_cpu, searchType, searchComparison, std::move(m_searchResults), searchStart, searchEnd, searchValue, searchHex ? 16 : 10);
workerWatcher->setFuture(workerFuture);
connect(workerWatcher, &QFutureWatcher<SearchResults>::finished, onSearchFinished);
m_searchResultsMap.clear();
connect(workerWatcher, &QFutureWatcher<std::vector<SearchResult>>::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<qsizetype>(m_ui.listSearchResults->count()) < m_searchResultsMap.size();
const bool hasResultsToLoad = static_cast<qsizetype>(m_ui.listSearchResults->count()) < m_searchResults.size();
const bool scrolledSufficiently = value > (m_ui.listSearchResults->verticalScrollBar()->maximum() * 0.95);
if (!m_resultsLoadTimer.isActive() && hasResultsToLoad && scrolledSufficiently)
{
Expand All @@ -612,18 +600,17 @@ 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;

const bool isFirstLoad = numLoaded == 0;
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);
Expand All @@ -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);
}
Expand All @@ -663,7 +650,7 @@ void MemorySearchWidget::updateSearchComparisonSelections()
const QString selectedComparisonLabel = m_ui.cmbSearchComparison->currentText();
const SearchComparison selectedComparison = m_searchComparisonLabelMap.labelToEnum(selectedComparisonLabel);

const std::vector<SearchComparison> comparisons = getValidSearchComparisonsForState(getCurrentSearchType(), m_searchResultsMap);
const std::vector<SearchComparison> comparisons = getValidSearchComparisonsForState(getCurrentSearchType(), m_searchResults);
m_ui.cmbSearchComparison->clear();
for (const SearchComparison comparison : comparisons)
{
Expand All @@ -677,14 +664,14 @@ void MemorySearchWidget::updateSearchComparisonSelections()
m_ui.cmbSearchComparison->setCurrentText(selectedComparisonLabel);
}

std::vector<SearchComparison> MemorySearchWidget::getValidSearchComparisonsForState(SearchType type, SearchResults existingResults)
std::vector<SearchComparison> MemorySearchWidget::getValidSearchComparisonsForState(SearchType type, std::vector<SearchResult>& existingResults)
{
const bool hasResults = existingResults.size() > 0;
std::vector<SearchComparison> 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);
Expand All @@ -698,7 +685,7 @@ std::vector<SearchComparison> 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);
Expand Down
4 changes: 2 additions & 2 deletions pcsx2-qt/Debugger/MemorySearchWidget.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public slots:
void switchToMemoryViewTab();

private:
QMap<u32, SearchResult> m_searchResultsMap;
std::vector<SearchResult> m_searchResults;
SearchComparisonLabelMap m_searchComparisonLabelMap;
Ui::MemorySearchWidget m_ui;
DebugInterface* m_cpu;
Expand All @@ -143,7 +143,7 @@ public slots:
u32 m_numResultsAddedPerLoad = 10000;

void updateSearchComparisonSelections();
std::vector<SearchComparison> getValidSearchComparisonsForState(SearchType type, QMap<u32, MemorySearchWidget::SearchResult> existingResults);
std::vector<SearchComparison> getValidSearchComparisonsForState(SearchType type, std::vector<SearchResult> &existingResults);
SearchType getCurrentSearchType();
SearchComparison getCurrentSearchComparison();
};

0 comments on commit 088405b

Please sign in to comment.