These patches add ability to start annotation browser from c2c report
tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
[1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
v1->v2: Addressed comments from Namhyung Kim
- No longer saving evsel for each hist entry, using evlist__first
instead.
- Factored out preparations to call annotation browser to do_annotate()
function
- Other small fixes and adjustments.
Artem Savkov (2):
perf util: move symbol__new_unresolved() to util/symbol.c
perf tools: allow running annotation browser from c2c-report
tools/perf/builtin-c2c.c | 73 +++++++++++++++++++++++++++++++---
tools/perf/ui/browsers/hists.c | 22 ----------
tools/perf/util/symbol.c | 22 ++++++++++
tools/perf/util/symbol.h | 1 +
4 files changed, 91 insertions(+), 27 deletions(-)
--
2.40.1
Add a shortcut to run annotation browser for selected symbol from
c2c report TUI.
Signed-off-by: Artem Savkov <[email protected]>
---
tools/perf/builtin-c2c.c | 73 +++++++++++++++++++++++++++++++++++++---
1 file changed, 68 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 05dfd98af170b..dce8604837aec 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -24,6 +24,7 @@
#include <perf/cpumap.h>
#include <subcmd/pager.h>
#include <subcmd/parse-options.h>
+#include "map.h"
#include "map_symbol.h"
#include "mem-events.h"
#include "session.h"
@@ -43,6 +44,8 @@
#include "ui/progress.h"
#include "pmus.h"
#include "string2.h"
+#include "util/annotate.h"
+#include "util/dso.h"
#include "util/util.h"
struct c2c_hists {
@@ -111,6 +114,9 @@ struct perf_c2c {
char *cl_sort;
char *cl_resort;
char *cl_output;
+
+ struct evlist *evlist;
+ struct annotation_options annotation_opts;
};
enum {
@@ -327,6 +333,9 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
c2c_he__set_node(c2c_he, sample);
hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
+ if (use_browser == 1 && al.map != NULL && al.sym != NULL)
+ hist_entry__inc_addr_samples(he, sample, evsel, al.addr);
+
ret = hist_entry__append_callchain(he, sample);
if (!ret) {
@@ -364,6 +373,9 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
c2c_he__set_node(c2c_he, sample);
hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
+ if (use_browser == 1 && al.map != NULL && al.sym != NULL)
+ hist_entry__inc_addr_samples(he, sample, evsel, al.addr);
+
ret = hist_entry__append_callchain(he, sample);
}
@@ -2612,6 +2624,43 @@ c2c_cacheline_browser__new(struct hists *hists, struct hist_entry *he)
return browser;
}
+static void do_annotate(struct hist_browser *browser, struct evsel *evsel) {
+ struct map_symbol ms = { NULL, NULL, NULL };
+
+ if (!browser->selection ||
+ !browser->selection->map ||
+ !browser->selection->map->dso ||
+ browser->selection->map->dso->annotate_warned) {
+ return;
+ }
+
+ ms.map = browser->selection->map;
+
+ if (!browser->selection->sym) {
+ if (!browser->he_selection)
+ return;
+
+ ms.sym = symbol__new_unresolved(browser->he_selection->ip,
+ browser->selection->map);
+
+ if (!ms.sym)
+ return;
+ } else {
+ if (symbol__annotation(browser->selection->sym)->src == NULL) {
+ ui_browser__warning(&browser->b, 0,
+ "No samples for the \"%s\" symbol.\n\n",
+ browser->selection->sym->name);
+ return;
+ }
+ ms.sym = browser->selection->sym;
+ }
+
+ map_symbol__tui_annotate(&ms, evsel, browser->hbt, &c2c.annotation_opts);
+
+ ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
+ ui_browser__handle_resize(&browser->b);
+}
+
static int perf_c2c__browse_cacheline(struct hist_entry *he)
{
struct c2c_hist_entry *c2c_he;
@@ -2621,6 +2670,7 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
int key = -1;
static const char help[] =
" ENTER Toggle callchains (if present) \n"
+ " a Annotate current symbol\n"
" n Toggle Node details info \n"
" s Toggle full length of symbol and source line columns \n"
" q Return back to cacheline list \n";
@@ -2650,6 +2700,9 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
key = hist_browser__run(browser, "? - help", true, 0);
switch (key) {
+ case 'a':
+ do_annotate(browser, evlist__first(c2c.evlist));
+ break;
case 's':
c2c.symbol_full = !c2c.symbol_full;
break;
@@ -3045,6 +3098,10 @@ static int perf_c2c__report(int argc, const char **argv)
int err = 0;
const char *output_str, *sort_str = NULL;
+ annotation_options__init(&c2c.annotation_opts);
+
+ hists__init();
+
argc = parse_options(argc, argv, options, report_c2c_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
if (argc)
@@ -3118,6 +3175,16 @@ static int perf_c2c__report(int argc, const char **argv)
if (err)
goto out_mem2node;
+ c2c.evlist = session->evlist;
+
+ if (c2c.use_stdio) {
+ use_browser = 0;
+ } else {
+ use_browser = 1;
+ symbol__annotation_init();
+ annotation_config__init(&c2c.annotation_opts);
+ }
+
if (symbol__init(&session->header.env) < 0)
goto out_mem2node;
@@ -3127,11 +3194,6 @@ static int perf_c2c__report(int argc, const char **argv)
goto out_mem2node;
}
- if (c2c.use_stdio)
- use_browser = 0;
- else
- use_browser = 1;
-
setup_browser(false);
err = perf_session__process_events(session);
@@ -3202,6 +3264,7 @@ static int perf_c2c__report(int argc, const char **argv)
out_session:
perf_session__delete(session);
out:
+ annotation_options__exit(&c2c.annotation_opts);
return err;
}
--
2.40.1
On Thu, Jun 08, 2023 at 10:44:05AM +0200, Artem Savkov wrote:
> These patches add ability to start annotation browser from c2c report
> tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
>
> [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
So what are the plans for doing IP to datatype::member resolution and
deleting this C2C abomination?
/me still regretting ever letting c2c happen..
Hi Peter,
On Thu, Jun 8, 2023 at 6:25 AM Peter Zijlstra <[email protected]> wrote:
>
> On Thu, Jun 08, 2023 at 10:44:05AM +0200, Artem Savkov wrote:
> > These patches add ability to start annotation browser from c2c report
> > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> >
> > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
>
> So what are the plans for doing IP to datatype::member resolution and
> deleting this C2C abomination?
I'm working on it and hoping to post an RFC soonish.. but no plan to kill
c2c entirely :)
Thanks,
Namhyung
>
> /me still regretting ever letting c2c happen..
Hello,
On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <[email protected]> wrote:
>
> These patches add ability to start annotation browser from c2c report
> tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
I was thinking about how it works and realized that it didn't collect
samples by symbol. Then I'm not sure if the result is meaningful.
I think it'd show a random symbol that touched the cache line
first. The same cache line can be accessed from other locations
but it cannot know where they are.
Also different instructions in a function (symbol) would access a
different cache line. The annotate output just shows any memory
access. So it might be good to check the instruction at the point
but others should not be considered related.
Hmm.. I suspect even the same instruction will hit the different
cache lines at different times. Then probably the annotation
won't work well in terms of correlating cache lines.
Thanks,
Namhyung
>
> [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
>
> v1->v2: Addressed comments from Namhyung Kim
> - No longer saving evsel for each hist entry, using evlist__first
> instead.
> - Factored out preparations to call annotation browser to do_annotate()
> function
> - Other small fixes and adjustments.
>
> Artem Savkov (2):
> perf util: move symbol__new_unresolved() to util/symbol.c
> perf tools: allow running annotation browser from c2c-report
>
> tools/perf/builtin-c2c.c | 73 +++++++++++++++++++++++++++++++---
> tools/perf/ui/browsers/hists.c | 22 ----------
> tools/perf/util/symbol.c | 22 ++++++++++
> tools/perf/util/symbol.h | 1 +
> 4 files changed, 91 insertions(+), 27 deletions(-)
>
> --
> 2.40.1
>
On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> Hello,
>
> On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <[email protected]> wrote:
> >
> > These patches add ability to start annotation browser from c2c report
> > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
>
> I was thinking about how it works and realized that it didn't collect
> samples by symbol. Then I'm not sure if the result is meaningful.
> I think it'd show a random symbol that touched the cache line
> first. The same cache line can be accessed from other locations
> but it cannot know where they are.
>
> Also different instructions in a function (symbol) would access a
> different cache line. The annotate output just shows any memory
> access. So it might be good to check the instruction at the point
> but others should not be considered related.
>
> Hmm.. I suspect even the same instruction will hit the different
> cache lines at different times. Then probably the annotation
> won't work well in terms of correlating cache lines.
The annotation hotkey is only added to the cacheline detailed view where
we do have symbol instruction information. The idea is to give the user
ability to quickly jump to source code/disassembly directly from c2c
TUI.
The hit percentages in annotation view don't make much sense in this
case though, so maybe it is better to use dummy evsel so that none are
shown.
> Thanks,
> Namhyung
>
> >
> > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
> >
> > v1->v2: Addressed comments from Namhyung Kim
> > - No longer saving evsel for each hist entry, using evlist__first
> > instead.
> > - Factored out preparations to call annotation browser to do_annotate()
> > function
> > - Other small fixes and adjustments.
> >
> > Artem Savkov (2):
> > perf util: move symbol__new_unresolved() to util/symbol.c
> > perf tools: allow running annotation browser from c2c-report
> >
> > tools/perf/builtin-c2c.c | 73 +++++++++++++++++++++++++++++++---
> > tools/perf/ui/browsers/hists.c | 22 ----------
> > tools/perf/util/symbol.c | 22 ++++++++++
> > tools/perf/util/symbol.h | 1 +
> > 4 files changed, 91 insertions(+), 27 deletions(-)
> >
> > --
> > 2.40.1
> >
>
--
Artem
Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > Hello,
> >
> > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <[email protected]> wrote:
> > >
> > > These patches add ability to start annotation browser from c2c report
> > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> >
> > I was thinking about how it works and realized that it didn't collect
> > samples by symbol. Then I'm not sure if the result is meaningful.
> > I think it'd show a random symbol that touched the cache line
> > first. The same cache line can be accessed from other locations
> > but it cannot know where they are.
> >
> > Also different instructions in a function (symbol) would access a
> > different cache line. The annotate output just shows any memory
> > access. So it might be good to check the instruction at the point
> > but others should not be considered related.
> >
> > Hmm.. I suspect even the same instruction will hit the different
> > cache lines at different times. Then probably the annotation
> > won't work well in terms of correlating cache lines.
>
> The annotation hotkey is only added to the cacheline detailed view where
> we do have symbol instruction information. The idea is to give the user
> ability to quickly jump to source code/disassembly directly from c2c
> TUI.
>
> The hit percentages in annotation view don't make much sense in this
> case though, so maybe it is better to use dummy evsel so that none are
> shown.
Yes, the point is just to reuse the source browser, if there is no
annotation data applicable, don't use any.
- Arnaldo
> > Thanks,
> > Namhyung
> >
> > >
> > > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
> > >
> > > v1->v2: Addressed comments from Namhyung Kim
> > > - No longer saving evsel for each hist entry, using evlist__first
> > > instead.
> > > - Factored out preparations to call annotation browser to do_annotate()
> > > function
> > > - Other small fixes and adjustments.
> > >
> > > Artem Savkov (2):
> > > perf util: move symbol__new_unresolved() to util/symbol.c
> > > perf tools: allow running annotation browser from c2c-report
> > >
> > > tools/perf/builtin-c2c.c | 73 +++++++++++++++++++++++++++++++---
> > > tools/perf/ui/browsers/hists.c | 22 ----------
> > > tools/perf/util/symbol.c | 22 ++++++++++
> > > tools/perf/util/symbol.h | 1 +
> > > 4 files changed, 91 insertions(+), 27 deletions(-)
> > >
> > > --
> > > 2.40.1
> > >
> >
>
> --
> Artem
>
--
- Arnaldo
Em Wed, Jun 14, 2023 at 03:38:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> > On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <[email protected]> wrote:
> > > > These patches add ability to start annotation browser from c2c report
> > > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> > > I was thinking about how it works and realized that it didn't collect
> > > samples by symbol. Then I'm not sure if the result is meaningful.
> > > I think it'd show a random symbol that touched the cache line
> > > first. The same cache line can be accessed from other locations
> > > but it cannot know where they are.
> > > Also different instructions in a function (symbol) would access a
> > > different cache line. The annotate output just shows any memory
> > > access. So it might be good to check the instruction at the point
> > > but others should not be considered related.
> > > Hmm.. I suspect even the same instruction will hit the different
> > > cache lines at different times. Then probably the annotation
> > > won't work well in terms of correlating cache lines.
> > The annotation hotkey is only added to the cacheline detailed view where
> > we do have symbol instruction information. The idea is to give the user
> > ability to quickly jump to source code/disassembly directly from c2c
> > TUI.
> > The hit percentages in annotation view don't make much sense in this
> > case though, so maybe it is better to use dummy evsel so that none are
> > shown.
> Yes, the point is just to reuse the source browser, if there is no
> annotation data applicable, don't use any.
I tried to work on having the annotation browser, both --stdio2 and the
TUI to work with evsels without annotation data, just so that we could
simply navigate source code or disassembly functions using the more
compact form we have in perf than the original one from objdump.
- Arnaldo
On Wed, Jun 14, 2023 at 03:38:35PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> > On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > > Hello,
> > >
> > > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <[email protected]> wrote:
> > > >
> > > > These patches add ability to start annotation browser from c2c report
> > > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> > >
> > > I was thinking about how it works and realized that it didn't collect
> > > samples by symbol. Then I'm not sure if the result is meaningful.
> > > I think it'd show a random symbol that touched the cache line
> > > first. The same cache line can be accessed from other locations
> > > but it cannot know where they are.
> > >
> > > Also different instructions in a function (symbol) would access a
> > > different cache line. The annotate output just shows any memory
> > > access. So it might be good to check the instruction at the point
> > > but others should not be considered related.
> > >
> > > Hmm.. I suspect even the same instruction will hit the different
> > > cache lines at different times. Then probably the annotation
> > > won't work well in terms of correlating cache lines.
> >
> > The annotation hotkey is only added to the cacheline detailed view where
> > we do have symbol instruction information. The idea is to give the user
> > ability to quickly jump to source code/disassembly directly from c2c
> > TUI.
> >
> > The hit percentages in annotation view don't make much sense in this
> > case though, so maybe it is better to use dummy evsel so that none are
> > shown.
>
> Yes, the point is just to reuse the source browser, if there is no
> annotation data applicable, don't use any.
>
Ok, should be as easy as the following diff. I'll include it in v3 if
there si more feedback.
---
diff --git b/tools/perf/builtin-c2c.c a/tools/perf/builtin-c2c.c
index dce8604837aec..c856ce7a50740 100644
--- b/tools/perf/builtin-c2c.c
+++ a/tools/perf/builtin-c2c.c
@@ -2701,7 +2701,11 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
switch (key) {
case 'a':
- do_annotate(browser, evlist__first(c2c.evlist));
+ /*
+ * We don't need percentage info so use 'dummy:HG'
+ * evsel which is last in evlist.
+ */
+ do_annotate(browser, evlist__last(c2c.evlist));
break;
case 's':
c2c.symbol_full = !c2c.symbol_full;