lkml.org 
[lkml]   [2020]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v5 1/4] perf util: Move block_pair_cmp to block-info
From
Date


On 1/31/2020 4:32 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 28, 2020 at 08:55:53PM +0800, Jin Yao escreveu:
>> block_pair_cmp() is a function which is used to compare
>> two blocks. Moving it from builtin-diff.c to block-info.c
>> to let it can be used by other builtins.
>>
>> v4/v5:
>> ------
>> No change.
>>
>> v3:
>> ---
>> Separate it from original patch for good tracking.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/builtin-diff.c | 17 -----------------
>> tools/perf/util/block-info.c | 17 +++++++++++++++++
>> tools/perf/util/block-info.h | 2 ++
>> 3 files changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
>> index f8b6ae557d8b..5ff1e21082cb 100644
>> --- a/tools/perf/builtin-diff.c
>> +++ b/tools/perf/builtin-diff.c
>> @@ -572,23 +572,6 @@ static void init_block_hist(struct block_hist *bh)
>> bh->valid = true;
>> }
>>
>> -static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b)
>> -{
>> - struct block_info *bi_a = a->block_info;
>> - struct block_info *bi_b = b->block_info;
>> - int cmp;
>> -
>> - if (!bi_a->sym || !bi_b->sym)
>> - return -1;
>> -
>> - cmp = strcmp(bi_a->sym->name, bi_b->sym->name);
>> -
>> - if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end))
>> - return 0;
>> -
>> - return -1;
>> -}
>> -
>> static struct hist_entry *get_block_pair(struct hist_entry *he,
>> struct hists *hists_pair)
>> {
>> diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
>> index c4b030bf6ec2..f0f38bdd496a 100644
>> --- a/tools/perf/util/block-info.c
>> +++ b/tools/perf/util/block-info.c
>> @@ -475,3 +475,20 @@ float block_info__total_cycles_percent(struct hist_entry *he)
>>
>> return 0.0;
>> }
>> +
>> +int block_pair_cmp(struct hist_entry *a, struct hist_entry *b)
>
> First thing that came to mind was that hist_entry comparision functions
> had been changed to return int64_t recently, when I went to look at it I
> found this:
>
> tools/perf/util/block-info.c
>
> int64_t block_info__cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> struct hist_entry *left, struct hist_entry *right)
> {
> struct block_info *bi_l = left->block_info;
> struct block_info *bi_r = right->block_info;
> int cmp;
> .
> .
> .
>
> Which look a bit more complete, can you check if that can be used
> instead or explain why my quick analysis of this is b0rken?
>
> Perhaps we can have a __block_info__cmp() that doesn't receive the
> perf_hpp_fmt (that isn't even used above) so that the previous use of
> block_pair_cmp() can be replaced with __block_info__cmp() instead?
>
> Thanks,
>
> - Arnaldo
>

Hi Arnaldo,

I think it's a good idea to use __block_info__cmp() to replace the
block_pair_cmp(). The block_info__cmp() is more comprehensive than
block_pair_cmp() and we only need a bit change such as,

--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -74,7 +74,7 @@ int64_t block_info__cmp(struct perf_hpp_fmt *fmt
__maybe_unused,

if (!bi_l->sym || !bi_r->sym) {
if (!bi_l->sym && !bi_r->sym)
- return 0;
+ return -1;
else if (!bi_l->sym)
return -1;
else
It returns -1 if both syms are NULL.

I will update the patchset.

Thanks
Jin Yao

>> +{
>> + struct block_info *bi_a = a->block_info;
>> + struct block_info *bi_b = b->block_info;
>> + int cmp;
>> +
>> + if (!bi_a->sym || !bi_b->sym)
>> + return -1;
>> +
>> + cmp = strcmp(bi_a->sym->name, bi_b->sym->name);
>> +
>> + if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end))
>> + return 0;
>> +
>> + return -1;
>> +}
>> diff --git a/tools/perf/util/block-info.h b/tools/perf/util/block-info.h
>> index bef0d75e9819..4fa91eeae92e 100644
>> --- a/tools/perf/util/block-info.h
>> +++ b/tools/perf/util/block-info.h
>> @@ -76,4 +76,6 @@ int report__browse_block_hists(struct block_hist *bh, float min_percent,
>>
>> float block_info__total_cycles_percent(struct hist_entry *he);
>>
>> +int block_pair_cmp(struct hist_entry *a, struct hist_entry *b);
>> +
>> #endif /* __PERF_BLOCK_H */
>> --
>> 2.17.1
>>
>

\
 
 \ /
  Last update: 2020-01-31 12:53    [W:0.060 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site