Re: [PATCH v2 0/2] perf tools: annotation browser from c2c tui

From: Artem Savkov <asavkov@redhat.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/2] perf tools: annotation browser from c2c tui
Date: Fri, 9 Jun 2023 10:37:31 +0200	[thread overview]
Message-ID: <20230609083731.GA168559@samus.usersys.redhat.com> (raw)
In-Reply-To: <CAM9d7cgvtuqMeQx8ehLCLw_Ur9Ju-VzDrakzFJxnxvFjdsWk_g@mail.gmail.com>

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 <asavkov@redhat.com> 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