Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cost attribution of inline frames in disassembly view #672

Merged
merged 1 commit into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions src/models/data.h
Original file line number Diff line number Diff line change
Expand Up @@ -683,18 +683,6 @@ struct CallerCalleeEntry
return *it;
}

LocationCost& offset(quint64 addr, int numTypes)
{
auto it = offsetMap.find(addr);
if (it == offsetMap.end()) {
it = offsetMap.insert(addr, {numTypes});
} else if (it->inclusiveCost.size() < static_cast<size_t>(numTypes)) {
it->inclusiveCost.resize(numTypes);
it->selfCost.resize(numTypes);
}
return *it;
}

ItemCost& callee(const Symbol& symbol, int numTypes)
{
auto it = callees.find(symbol);
Expand All @@ -719,14 +707,14 @@ struct CallerCalleeEntry
CalleeMap callees;
// source map for this symbol, i.e. locations mapped to associated costs
SourceLocationCostMap sourceMap;
// per-IP map for this symbol for disassembly
OffsetLocationCostMap offsetMap;
};

using CallerCalleeEntryMap = QHash<Symbol, CallerCalleeEntry>;
struct CallerCalleeResults
{
CallerCalleeEntryMap entries;
// per-binary map of per-IP (relAddr) map for disassembly
QHash<QString, OffsetLocationCostMap> binaryOffsetMap;
Costs selfCosts;
Costs inclusiveCosts;

Expand All @@ -739,6 +727,23 @@ struct CallerCalleeResults
}
return *it;
}

LocationCost& binaryOffset(const QString& binary, quint64 addr, int numTypes)
{
auto binaryIt = binaryOffsetMap.find(binary);
if (binaryIt == binaryOffsetMap.end()) {
binaryIt = binaryOffsetMap.insert(binary, {});
}

auto it = binaryIt->find(addr);
if (it == binaryIt->end()) {
it = binaryIt->insert(addr, {numTypes});
} else if (it->inclusiveCost.size() < static_cast<size_t>(numTypes)) {
it->inclusiveCost.resize(numTypes);
it->selfCost.resize(numTypes);
}
return *it;
}
};

void callerCalleesFromBottomUpData(const BottomUpResults& data, CallerCalleeResults* results);
Expand Down
11 changes: 6 additions & 5 deletions src/models/disassemblymodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ void DisassemblyModel::clear()
{
beginResetModel();
m_data = {};
m_offsetMap = {};
endResetModel();
}

Expand All @@ -47,6 +48,7 @@ void DisassemblyModel::setDisassembly(const DisassemblyOutput& disassemblyOutput

m_data = disassemblyOutput;
m_results = results;
m_offsetMap = m_results.binaryOffsetMap[m_data.symbol.binary];
m_numTypes = results.selfCosts.numTypes();

QStringList assemblyLines;
Expand Down Expand Up @@ -128,9 +130,8 @@ QVariant DisassemblyModel::data(const QModelIndex& index, int role) const
return {};
}

const auto entry = m_results.entries.value(m_data.symbol);
auto it = entry.offsetMap.find(data.addr);
if (it != entry.offsetMap.end()) {
auto it = m_offsetMap.find(data.addr);
if (it != m_offsetMap.end()) {
const auto event = index.column() - COLUMN_COUNT;
const auto& locationCost = it.value();

Expand Down Expand Up @@ -223,8 +224,8 @@ QModelIndex DisassemblyModel::indexForFileLine(const Data::FileLine& fileLine) c
bestMatch = i;
}

auto it = entry.offsetMap.find(line.addr);
if (it != entry.offsetMap.end()) {
auto it = m_offsetMap.find(line.addr);
if (it != m_offsetMap.end()) {
const auto& locationCost = it.value();

if (!bestCost || bestCost < locationCost.selfCost[0]) {
Expand Down
1 change: 1 addition & 0 deletions src/models/disassemblymodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public slots:
HighlightedText m_highlightedText;
DisassemblyOutput m_data;
Data::CallerCalleeResults m_results;
Data::OffsetLocationCostMap m_offsetMap;
int m_numTypes = 0;
int m_highlightLine = 0;
};
3 changes: 2 additions & 1 deletion src/parsers/perf/perfparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,8 @@ void addCallerCalleeEvent(const Data::Symbol& symbol, const Data::Location& loca
auto& entry = callerCalleeResult->entry(symbol);
auto& sourceCost = entry.source(location.fileLine, numCosts);
// relAddr can be 0 for symbols in the main executable
auto& addrCost = entry.offset(location.relAddr ? location.relAddr : location.address, numCosts);
auto& addrCost = callerCalleeResult->binaryOffset(
symbol.binary, location.relAddr ? location.relAddr : location.address, numCosts);

sourceCost.inclusiveCost[type] += cost;
addrCost.inclusiveCost[type] += cost;
Expand Down
3 changes: 1 addition & 2 deletions tests/modeltests/tst_models.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ private slots:
Data::CallerCalleeResults results;
Data::callerCalleesFromBottomUpData(tree, &results);

auto entry = results.entry(symbol);
auto& locationCost = entry.offset(4294563, results.selfCosts.numTypes());
auto& locationCost = results.binaryOffset(symbol.binary, 4294563, results.selfCosts.numTypes());
locationCost.inclusiveCost[0] += 200;
locationCost.selfCost[0] += 200;

Expand Down