From 794c33091121f6c0b3488f2484866fe0c176966a Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Mon, 22 Jul 2024 14:30:37 +0100 Subject: [PATCH] ui: add property support to flamegraph and bring back mapping name This CL adds support to flamegraph widget to display arbitrary properties on every node. It also adds support for perf samples and native heap profiles to show mapping names when a node is clicked on which brings it back to parity with old flamegraph. Finally, it also adds support for showing file name/line number if those are non-null. Fixes: https://github.com/google/perfetto/issues/846 Change-Id: I16610a61d6b1aa6972b63ec5e47a6c55d2a21b51 --- .../importers/proto/profile_module.cc | 13 +++++-- .../memory/heap_profile/callstacks.sql | 3 ++ .../stdlib/callstacks/stack_profile.sql | 7 +++- .../stdlib/linux/perf/samples.sql | 3 ++ src/trace_processor/tables/profiler_tables.py | 4 +- src/trace_processor/util/profile_builder.cc | 6 ++- ui/src/core/query_flamegraph.ts | 37 +++++++++++++++---- ui/src/core_plugins/heap_profile/index.ts | 8 ++++ .../perf_samples_profile/index.ts | 23 +++++++++++- ui/src/widgets/flamegraph.ts | 10 ++++- 10 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/trace_processor/importers/proto/profile_module.cc b/src/trace_processor/importers/proto/profile_module.cc index ab44d2a3e3..a5e3672d92 100644 --- a/src/trace_processor/importers/proto/profile_module.cc +++ b/src/trace_processor/importers/proto/profile_module.cc @@ -15,6 +15,7 @@ */ #include "src/trace_processor/importers/proto/profile_module.h" +#include #include #include "perfetto/base/logging.h" @@ -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) { diff --git a/src/trace_processor/perfetto_sql/stdlib/android/memory/heap_profile/callstacks.sql b/src/trace_processor/perfetto_sql/stdlib/android/memory/heap_profile/callstacks.sql index 6167c589e1..2867ad7c87 100644 --- a/src/trace_processor/perfetto_sql/stdlib/android/memory/heap_profile/callstacks.sql +++ b/src/trace_processor/perfetto_sql/stdlib/android/memory/heap_profile/callstacks.sql @@ -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, diff --git a/src/trace_processor/perfetto_sql/stdlib/callstacks/stack_profile.sql b/src/trace_processor/perfetto_sql/stdlib/callstacks/stack_profile.sql index c5378d9292..702151bb91 100644 --- a/src/trace_processor/perfetto_sql/stdlib/callstacks/stack_profile.sql +++ b/src/trace_processor/perfetto_sql/stdlib/callstacks/stack_profile.sql @@ -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, ( @@ -108,4 +110,5 @@ AS ) ) g JOIN _callstack_spc_forest f USING (id) + JOIN stack_profile_mapping m ON f.mapping_id = m.id ); diff --git a/src/trace_processor/perfetto_sql/stdlib/linux/perf/samples.sql b/src/trace_processor/perfetto_sql/stdlib/linux/perf/samples.sql index c2c84caffe..1a22bc115e 100644 --- a/src/trace_processor/perfetto_sql/stdlib/linux/perf/samples.sql +++ b/src/trace_processor/perfetto_sql/stdlib/linux/perf/samples.sql @@ -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) diff --git a/src/trace_processor/tables/profiler_tables.py b/src/trace_processor/tables/profiler_tables.py index 88d114bd55..2c55640755 100644 --- a/src/trace_processor/tables/profiler_tables.py +++ b/src/trace_processor/tables/profiler_tables.py @@ -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=''' diff --git a/src/trace_processor/util/profile_builder.cc b/src/trace_processor/util/profile_builder.cc index 6948a8e8e7..2e02a845e3 100644 --- a/src/trace_processor/util/profile_builder.cc +++ b/src/trace_processor/util/profile_builder.cc @@ -480,7 +480,7 @@ std::vector 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)}); } } @@ -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}); diff --git a/ui/src/core/query_flamegraph.ts b/ui/src/core/query_flamegraph.ts index bd1f3fc4e2..2d957c1250 100644 --- a/ui/src/core/query_flamegraph.ts +++ b/ui/src/core/query_flamegraph.ts @@ -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, @@ -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; @@ -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, + aggregatableProperties?: ReadonlyArray, ): QueryFlamegraphMetric[] { const metrics = []; for (const {name, unit, columnName} of tableMetrics) { @@ -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; @@ -170,7 +177,7 @@ async function computeFlamegraphTree( aggregatableProperties, }: QueryFlamegraphMetric, {showStack, hideStack, showFromFrame, hideFrame, pivot}: FlamegraphFilters, -) { +): Promise { // Pivot also essentially acts as a "show stack" filter so treat it like one. const showStackAndPivot = [...showStack]; if (pivot !== undefined) { @@ -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); @@ -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}), @@ -337,6 +347,7 @@ async function computeFlamegraphTree( ${groupedColumns} ) `); + const it = res.iter({ id: NUM, parentId: NUM, @@ -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(); + 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, @@ -361,6 +381,7 @@ async function computeFlamegraphTree( cumulativeValue: it.cumulativeValue, xStart: it.xStart, xEnd: it.xEnd, + properties, }); if (it.depth === 1) { allRootsCumulativeValue += it.cumulativeValue; diff --git a/ui/src/core_plugins/heap_profile/index.ts b/ui/src/core_plugins/heap_profile/index.ts index be326249f5..a4ade68c2b 100644 --- a/ui/src/core_plugins/heap_profile/index.ts +++ b/ui/src/core_plugins/heap_profile/index.ts @@ -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, @@ -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'}, + ], ), ], }; diff --git a/ui/src/core_plugins/perf_samples_profile/index.ts b/ui/src/core_plugins/perf_samples_profile/index.ts index c054d65e75..dc7451eba9 100644 --- a/ui/src/core_plugins/perf_samples_profile/index.ts +++ b/ui/src/core_plugins/perf_samples_profile/index.ts @@ -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 @@ -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 @@ -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'}, + ], ), ], }; diff --git a/ui/src/widgets/flamegraph.ts b/ui/src/widgets/flamegraph.ts index f0dbd46895..8bb22f161c 100644 --- a/ui/src/widgets/flamegraph.ts +++ b/ui/src/widgets/flamegraph.ts @@ -88,6 +88,7 @@ export interface FlamegraphQueryData { readonly name: string; readonly selfValue: number; readonly cumulativeValue: number; + readonly properties: ReadonlyMap; readonly xStart: number; readonly xEnd: number; }>; @@ -542,7 +543,7 @@ export class Flamegraph implements m.ClassComponent { ); } 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)); @@ -562,6 +563,13 @@ export class Flamegraph implements m.ClassComponent { 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, {},