Re: [PATCH 4/4] perf tools: Compare hists comm by addresses

From: Namhyung Kim
Date: Fri Sep 13 2013 - 04:07:21 EST


Hi,

On Thu, 12 Sep 2013 22:29:43 +0200, Frederic Weisbecker wrote:
> Now that comm strings are allocated only once and refcounted to be shared
> among threads, these can now be safely compared by addresses. This
> should remove most hists collapses on post processing.

[SNIP]

> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 26d8f11..3b307e7 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -79,7 +79,8 @@ struct sort_entry sort_thread = {
> static int64_t
> sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - return right->thread->tid - left->thread->tid;
> + /* Compare the addr that should be unique among comm */
> + return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);

I don't think this is enough. A hist entry should reference the comm
itself at that time. Saving thread can lead to referencing different
comm between insert and collapse time if thread changed the comm in the
meanwhile, right?

What about something like below: