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

[Namhyung Kim]

Hi Frederic,

On Fri, 13 Sep 2013 15:59:49 +0200, Frederic Weisbecker wrote:
> On Fri, Sep 13, 2013 at 05:07:06PM +0900, Namhyung Kim wrote:

[SNIP]

>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -1,5 +1,6 @@
>>  #include "sort.h"
>>  #include "hist.h"
>> +#include "comm.h"
>>  #include "symbol.h"
>>  
>>  regex_t		parent_regex;
>> @@ -80,14 +81,14 @@ static int64_t
>>  sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
>>  {
>>  	/* Compare the addr that should be unique among comm */
>> -	return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);
>> +	return comm__str(right->comm) - comm__str(left->comm);
>
>
> If comm and fork events don't have a timestamp, or they aren't time ordered, we should
> override the comm entry of a thread and forget the previous one.
>
> So perhaps what we should do instead is to compare "struct comm" addresses directly.
> But it means that on thread__set_comm(), if we override the old entry due to missing or
> unordered timestamps (in which case we need to force them to be 0), we shouldn't reallocate
> a new struct comm, nor should we keep the old one and queue a new. Instead we need to override
> list_first_entry(thread->comm)->comm_str pointer only to make it point to a new struct comm_str.

Okay.  Here's a revised patch: