Skip to content

Commit

Permalink
Merge "ui: add property support to flamegraph and bring back mapping …
Browse files Browse the repository at this point in the history
…name" into main
  • Loading branch information
LalitMaganti authored and Gerrit Code Review committed Jul 22, 2024
2 parents 77df378 + 794c330 commit f37e595
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 21 deletions.
13 changes: 9 additions & 4 deletions src/trace_processor/importers/proto/profile_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "src/trace_processor/importers/proto/profile_module.h"
#include <optional>
#include <string>

#include "perfetto/base/logging.h"
Expand Down Expand Up @@ -454,13 +455,17 @@ void ProfileModule::ParseModuleSymbols(ConstBytes blob) {
ArgsTranslationTable::SourceLocation last_location;
for (auto line_it = address_symbols.lines(); line_it; ++line_it) {
protos::pbzero::Line::Decoder line(*line_it);
auto file_name = line.source_file_name();
context_->storage->mutable_symbol_table()->Insert(
{symbol_set_id, context_->storage->InternString(line.function_name()),
context_->storage->InternString(line.source_file_name()),
line.line_number()});
file_name.size == 0 ? kNullStringId
: context_->storage->InternString(file_name),
line.has_line_number() && file_name.size != 0
? std::make_optional(line.line_number())
: std::nullopt});
last_location = ArgsTranslationTable::SourceLocation{
line.source_file_name().ToStdString(),
line.function_name().ToStdString(), line.line_number()};
file_name.ToStdString(), line.function_name().ToStdString(),
line.line_number()};
has_lines = true;
}
if (!has_lines) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ AS
c.id,
c.parent_id,
c.name,
c.mapping_name,
c.source_file,
c.line_number,
IFNULL(m.self_size, 0) AS self_size,
IFNULL(m.self_count, 0) AS self_count,
IFNULL(m.self_alloc_size, 0) AS self_alloc_size,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,11 @@ AS
SELECT
f.id,
f.parent_id,
f.name,
f.callsite_id,
f.is_leaf_function_in_callsite_frame
f.name,
m.name AS mapping_name,
f.source_file,
f.line_number
FROM _tree_reachable_ancestors_or_self!(
_callstack_spc_forest,
(
Expand All @@ -108,4 +110,5 @@ AS
)
) g
JOIN _callstack_spc_forest f USING (id)
JOIN stack_profile_mapping m ON f.mapping_id = m.id
);
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ AS
c.id,
c.parent_id,
c.name,
c.mapping_name,
c.source_file,
c.line_number,
IFNULL(m.self_count, 0) AS self_count
FROM _callstacks_for_stack_profile_samples!(metrics) c
LEFT JOIN metrics m USING (callsite_id)
Expand Down
4 changes: 2 additions & 2 deletions src/trace_processor/tables/profiler_tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@
CppUint32(),
flags=ColumnFlag.SORTED | ColumnFlag.SET_ID),
C('name', CppString()),
C('source_file', CppString()),
C('line_number', CppUint32()),
C('source_file', CppOptional(CppString())),
C('line_number', CppOptional(CppUint32())),
],
tabledoc=TableDoc(
doc='''
Expand Down
6 changes: 4 additions & 2 deletions src/trace_processor/util/profile_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ std::vector<GProfileBuilder::Line> GProfileBuilder::GetLinesForSymbolSetId(
if (uint64_t function_id =
WriteFunctionIfNeeded(symbol, annotation, mapping_id);
function_id != kNullFunctionId) {
lines.push_back({function_id, symbol.line_number()});
lines.push_back({function_id, symbol.line_number().value_or(0)});
}
}

Expand All @@ -502,7 +502,9 @@ uint64_t GProfileBuilder::WriteFunctionIfNeeded(
CallsiteAnnotation annotation,
uint64_t mapping_id) {
int64_t name = string_table_.GetAnnotatedString(symbol.name(), annotation);
int64_t filename = string_table_.InternString(symbol.source_file());
int64_t filename = symbol.source_file().has_value()
? string_table_.InternString(*symbol.source_file())
: kEmptyStringIndex;

auto ins = functions_.insert(
{Function{name, kEmptyStringIndex, filename}, functions_.size() + 1});
Expand Down
37 changes: 29 additions & 8 deletions ui/src/core/query_flamegraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import m from 'mithril';

import {AsyncDisposableStack} from '../base/disposable_stack';
import {Engine} from '../trace_processor/engine';
import {NUM, STR} from '../trace_processor/query_result';
import {NUM, STR, STR_NULL} from '../trace_processor/query_result';
import {createPerfettoTable} from '../trace_processor/sql_utils';
import {
Flamegraph,
Expand Down Expand Up @@ -68,6 +68,8 @@ export interface QueryFlamegraphMetric {
// the user if there is no merging. If there is merging, currently the value
// will not be shown.
//
// Examples include the source file and line number.
//
// TODO(lalitm): reconsider the decision to show nothing, instead maybe show
// the top 5 options etc.
readonly aggregatableProperties?: ReadonlyArray<QueryFlamegraphColumn>;
Expand All @@ -78,11 +80,14 @@ export interface QueryFlamegraphMetric {
// in QueryFlamegraph's attrs.
//
// `tableOrSubquery` should have the columns `id`, `parentId`, `name` and all
// columns specified by `tableMetrics[].name`.
// columns specified by `tableMetrics[].name`, `unaggregatableProperties` and
// `aggregatableProperties`.
export function metricsFromTableOrSubquery(
tableOrSubquery: string,
tableMetrics: ReadonlyArray<{name: string; unit: string; columnName: string}>,
dependencySql?: string,
unaggregatableProperties?: ReadonlyArray<QueryFlamegraphColumn>,
aggregatableProperties?: ReadonlyArray<QueryFlamegraphColumn>,
): QueryFlamegraphMetric[] {
const metrics = [];
for (const {name, unit, columnName} of tableMetrics) {
Expand All @@ -91,9 +96,11 @@ export function metricsFromTableOrSubquery(
unit,
dependencySql,
statement: `
select id, parentId, name, ${columnName} as value
select *, ${columnName} as value
from ${tableOrSubquery}
`,
unaggregatableProperties,
aggregatableProperties,
});
}
return metrics;
Expand Down Expand Up @@ -170,7 +177,7 @@ async function computeFlamegraphTree(
aggregatableProperties,
}: QueryFlamegraphMetric,
{showStack, hideStack, showFromFrame, hideFrame, pivot}: FlamegraphFilters,
) {
): Promise<FlamegraphQueryData> {
// Pivot also essentially acts as a "show stack" filter so treat it like one.
const showStackAndPivot = [...showStack];
if (pivot !== undefined) {
Expand Down Expand Up @@ -206,10 +213,13 @@ async function computeFlamegraphTree(
const pivotFilter = pivot === undefined ? '0' : `name like '%${pivot}%'`;

const unagg = unaggregatableProperties ?? [];
const unaggCols = unagg.map((x) => x.name);

const agg = aggregatableProperties ?? [];
const aggCols = agg.map((x) => x.name);

const groupingColumns = `(${(unagg.length === 0 ? ['groupingColumn'] : unagg).join()})`;
const groupedColumns = `(${(agg.length === 0 ? ['groupedColumn'] : agg).join()})`;
const groupingColumns = `(${(unaggCols.length === 0 ? ['groupingColumn'] : unaggCols).join()})`;
const groupedColumns = `(${(aggCols.length === 0 ? ['groupedColumn'] : aggCols).join()})`;

if (dependencySql !== undefined) {
await engine.query(dependencySql);
Expand All @@ -233,8 +243,8 @@ async function computeFlamegraphTree(
parentId,
name,
value,
${(unagg.length === 0 ? ['0 as groupingColumn'] : unagg).join()},
${(agg.length === 0 ? ['0 as groupedColumn'] : agg).join()}
${(unaggCols.length === 0 ? [`'' as groupingColumn`] : unaggCols).join()},
${(aggCols.length === 0 ? [`'' as groupedColumn`] : aggCols).join()}
FROM (${statement})
),
(${showStackFilter}),
Expand Down Expand Up @@ -337,6 +347,7 @@ async function computeFlamegraphTree(
${groupedColumns}
)
`);

const it = res.iter({
id: NUM,
parentId: NUM,
Expand All @@ -346,12 +357,21 @@ async function computeFlamegraphTree(
cumulativeValue: NUM,
xStart: NUM,
xEnd: NUM,
...Object.fromEntries(unaggCols.map((m) => [m, STR_NULL])),
...Object.fromEntries(aggCols.map((m) => [m, STR_NULL])),
});
let allRootsCumulativeValue = 0;
let minDepth = 0;
let maxDepth = 0;
const nodes = [];
for (; it.valid(); it.next()) {
const properties = new Map<string, string>();
for (const a of [...agg, ...unagg]) {
const r = it.get(a.name);
if (r !== null) {
properties.set(a.displayName, r as string);
}
}
nodes.push({
id: it.id,
parentId: it.parentId,
Expand All @@ -361,6 +381,7 @@ async function computeFlamegraphTree(
cumulativeValue: it.cumulativeValue,
xStart: it.xStart,
xEnd: it.xEnd,
properties,
});
if (it.depth === 1) {
allRootsCumulativeValue += it.cumulativeValue;
Expand Down
8 changes: 8 additions & 0 deletions ui/src/core_plugins/heap_profile/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ function flamegraphAttrsForHeapProfile(
id,
parent_id as parentId,
name,
mapping_name,
source_file,
cast(line_number AS text) as line_number,
self_size,
self_count,
self_alloc_size,
Expand All @@ -306,6 +309,11 @@ function flamegraphAttrsForHeapProfile(
`,
metrics,
'include perfetto module android.memory.heap_profile.callstacks',
[{name: 'mapping_name', displayName: 'Mapping'}],
[
{name: 'source_file', displayName: 'Source File'},
{name: 'line_number', displayName: 'Line Number'},
],
),
],
};
Expand Down
23 changes: 21 additions & 2 deletions ui/src/core_plugins/perf_samples_profile/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,14 @@ class PerfSamplesFlamegraphDetailsPanel implements LegacyDetailsPanel {
upid === undefined
? `
(
select id, parent_id as parentId, name, self_count
select
id,
parent_id as parentId,
name,
mapping_name,
source_file,
cast(line_number AS text) as line_number,
self_count
from _linux_perf_callstacks_for_samples!((
select p.callsite_id
from perf_sample p
Expand All @@ -177,7 +184,14 @@ class PerfSamplesFlamegraphDetailsPanel implements LegacyDetailsPanel {
`
: `
(
select id, parent_id as parentId, name, self_count
select
id,
parent_id as parentId,
name,
mapping_name,
source_file,
cast(line_number AS text) as line_number,
self_count
from _linux_perf_callstacks_for_samples!((
select p.callsite_id
from perf_sample p
Expand All @@ -196,6 +210,11 @@ class PerfSamplesFlamegraphDetailsPanel implements LegacyDetailsPanel {
},
],
'include perfetto module linux.perf.samples',
[{name: 'mapping_name', displayName: 'Mapping'}],
[
{name: 'source_file', displayName: 'Source File'},
{name: 'line_number', displayName: 'Line Number'},
],
),
],
};
Expand Down
10 changes: 9 additions & 1 deletion ui/src/widgets/flamegraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface FlamegraphQueryData {
readonly name: string;
readonly selfValue: number;
readonly cumulativeValue: number;
readonly properties: ReadonlyMap<string, string>;
readonly xStart: number;
readonly xEnd: number;
}>;
Expand Down Expand Up @@ -542,7 +543,7 @@ export class Flamegraph implements m.ClassComponent<FlamegraphAttrs> {
);
}
const {queryIdx} = node.source;
const {name, cumulativeValue, selfValue} = nodes[queryIdx];
const {name, cumulativeValue, selfValue, properties} = nodes[queryIdx];
const filterButtonClick = (filter: string) => {
this.rawFilters = addFilter(this.rawFilters, filter);
this.attrs.onFiltersChanged(computeFilters(this.rawFilters));
Expand All @@ -562,6 +563,13 @@ export class Flamegraph implements m.ClassComponent<FlamegraphAttrs> {
m('.tooltip-bold-text', 'Self:'),
m('.tooltip-text', displaySize(selfValue, unit)),
),
Array.from(properties, ([key, value]) => {
return m(
'.tooltip-text-line',
m('.tooltip-bold-text', key),
m('.tooltip-text', value),
);
}),
m(
ButtonBar,
{},
Expand Down

0 comments on commit f37e595

Please sign in to comment.