linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: kan.liang@intel.com
Cc: jolsa@redhat.com, namhyung@kernel.org,
	linux-kernel@vger.kernel.org, ak@linux.intel.com
Subject: Re: [PATCH V4 2/3] perf tool:perf diff support for different binaries
Date: Tue, 18 Nov 2014 18:20:02 -0300	[thread overview]
Message-ID: <20141118212002.GE3790@kernel.org> (raw)
In-Reply-To: <1416328700-1836-3-git-send-email-kan.liang@intel.com>

Em Tue, Nov 18, 2014 at 11:38:19AM -0500, kan.liang@intel.com escreveu:
> From: Kan Liang <kan.liang@intel.com>
> 
> Currently, the perf diff only works with same binaries. That's because
> it compares the symbol start address. It doesn't work if the perf.data
> comes from different binaries. This patch matches the function names.
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Can you provide an example of this?

Also, can you add some note on the documentation? The example you use in
the changelog may also be appropriate to have in the man page.

Its been a looong while since I looked at the diff code, but yeah,
comparing by symbol address makes no sense, it had to do a diff from
file name!

The canonical example is:

  vim workload.c
  make workload
  perf record ./workload
  vim workload.c # do some changes
  perf record ./workload
  perf diff

And of course doing that would probably result in symbols moving around
as functions grow in size and new ones are added...

/me scratches head...

- Arnaldo

> ---
>  tools/perf/util/sort.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 95efaaf..bea2e07 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1432,6 +1432,15 @@ int sort_dimension__add(const char *tok)
>  			sort__has_parent = 1;
>  		} else if (sd->entry == &sort_sym) {
>  			sort__has_sym = 1;
> +			/*
> +			 * perf diff displays the performance difference amongst
> +			 * two or more perf.data files. Those files could come
> +			 * from different binaries. So we should not compare
> +			 * their ips, but the name of symble.

					s/symble/symbol/g

> +			 */
> +			if (sort__mode == SORT_MODE__DIFF)
> +				sd->entry->se_collapse = sort__sym_sort;
> +
>  		} else if (sd->entry == &sort_dso) {
>  			sort__has_dso = 1;
>  		}
> -- 
> 1.8.3.2

  reply	other threads:[~2014-11-19 15:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18 16:38 [PATCH V4 0/3] perf tool: perf diff sort changes kan.liang
2014-11-18 16:38 ` [PATCH V4 1/3] perf tool: Fix perf diff symble sort issue kan.liang
2014-11-18 21:11   ` Arnaldo Carvalho de Melo
2014-11-20  7:39   ` [tip:perf/core] perf diff: Add missing handler for PERF_RECORD_MMAP2 events tip-bot for Kan Liang
2014-11-18 16:38 ` [PATCH V4 2/3] perf tool:perf diff support for different binaries kan.liang
2014-11-18 21:20   ` Arnaldo Carvalho de Melo [this message]
2014-11-18 16:38 ` [PATCH V4 3/3] perf tool: Add sort key symoff for perf diff kan.liang
2014-11-18 21:13   ` Arnaldo Carvalho de Melo
2014-11-19 20:44     ` Liang, Kan
2014-11-20 20:50       ` Arnaldo Carvalho de Melo
2014-11-20  6:18     ` Namhyung Kim
2014-11-19  6:46   ` Namhyung Kim
2014-11-19 14:17     ` Liang, Kan
2014-11-20  6:24       ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141118212002.GE3790@kernel.org \
    --to=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).