2020-12-13 18:47:23

by Leo Yan

Subject: [PATCH v2 06/11] perf c2c: Refactor display filter macro

When sort on the respective metrics (lcl_hitm, rmt_hitm, tot_hitm),
macro FILTER_HITM is to filter out the cache line entries if its
overhead is less than 1%.

This patch is to refactor macro FILTER_HITM. It uses more gernal name
FILTER_DISPLAY to replace the old name; and refines its parameter,
rather than passing field name for the data structure, it changes to
pass the cache line's statistic value and the sum value, this is more
flexsible, e.g. if consider to extend for sorting on all load hits
which combines multiple fields from structure c2c_stats.

Signed-off-by: Leo Yan <[email protected]>
---
tools/perf/builtin-c2c.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 5cd30c083d6c..f11c3c84bb2b 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2151,24 +2151,27 @@ static bool he__display(struct hist_entry *he, struct c2c_stats *stats)

c2c_he = container_of(he, struct c2c_hist_entry, he);

-#define FILTER_HITM(__h) \
- if (stats->__h) { \
- ld_dist = ((double)c2c_he->stats.__h / stats->__h); \
+#define FILTER_DISPLAY(val, sum) \
+{ \
+ if ((sum)) { \
+ ld_dist = ((double)(val) / (sum)); \
if (ld_dist < DISPLAY_LINE_LIMIT) \
he->filtered = HIST_FILTER__C2C; \
} else { \
he->filtered = HIST_FILTER__C2C; \
- }
+ } \
+}

switch (c2c.display) {
case DISPLAY_LCL:
- FILTER_HITM(lcl_hitm);
+ FILTER_DISPLAY(c2c_he->stats.lcl_hitm, stats->lcl_hitm);
break;
case DISPLAY_RMT:
- FILTER_HITM(rmt_hitm);
+ FILTER_DISPLAY(c2c_he->stats.rmt_hitm, stats->rmt_hitm);
break;
case DISPLAY_TOT:
- FILTER_HITM(tot_hitm);
+ FILTER_DISPLAY(c2c_he->stats.tot_hitm, stats->tot_hitm);
+ break;
default:
break;
}
--
2.17.1

2021-01-06 07:49:11

by Namhyung Kim

Subject: Re: [PATCH v2 06/11] perf c2c: Refactor display filter macro

On Sun, Dec 13, 2020 at 10:39 PM Leo Yan <[email protected]> wrote:
>
> When sort on the respective metrics (lcl_hitm, rmt_hitm, tot_hitm),
> macro FILTER_HITM is to filter out the cache line entries if its
> overhead is less than 1%.
>
> This patch is to refactor macro FILTER_HITM. It uses more gernal name
> FILTER_DISPLAY to replace the old name; and refines its parameter,
> rather than passing field name for the data structure, it changes to
> pass the cache line's statistic value and the sum value, this is more
> flexsible, e.g. if consider to extend for sorting on all load hits
> which combines multiple fields from structure c2c_stats.

As it doesn't use field names anymore, I think it's better to change it to
a static function.

Thanks,
Namhyung


>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> tools/perf/builtin-c2c.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 5cd30c083d6c..f11c3c84bb2b 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -2151,24 +2151,27 @@ static bool he__display(struct hist_entry *he, struct c2c_stats *stats)
>
> c2c_he = container_of(he, struct c2c_hist_entry, he);
>
> -#define FILTER_HITM(__h) \
> - if (stats->__h) { \
> - ld_dist = ((double)c2c_he->stats.__h / stats->__h); \
> +#define FILTER_DISPLAY(val, sum) \
> +{ \
> + if ((sum)) { \
> + ld_dist = ((double)(val) / (sum)); \
> if (ld_dist < DISPLAY_LINE_LIMIT) \
> he->filtered = HIST_FILTER__C2C; \
> } else { \
> he->filtered = HIST_FILTER__C2C; \
> - }
> + } \
> +}
>
> switch (c2c.display) {
> case DISPLAY_LCL:
> - FILTER_HITM(lcl_hitm);
> + FILTER_DISPLAY(c2c_he->stats.lcl_hitm, stats->lcl_hitm);
> break;
> case DISPLAY_RMT:
> - FILTER_HITM(rmt_hitm);
> + FILTER_DISPLAY(c2c_he->stats.rmt_hitm, stats->rmt_hitm);
> break;
> case DISPLAY_TOT:
> - FILTER_HITM(tot_hitm);
> + FILTER_DISPLAY(c2c_he->stats.tot_hitm, stats->tot_hitm);
> + break;
> default:
> break;
> }
> --
> 2.17.1
>

2021-01-11 08:47:09

by Leo Yan

Subject: Re: [PATCH v2 06/11] perf c2c: Refactor display filter macro

On Wed, Jan 06, 2021 at 04:47:00PM +0900, Namhyung Kim wrote:
> On Sun, Dec 13, 2020 at 10:39 PM Leo Yan <[email protected]> wrote:
> >
> > When sort on the respective metrics (lcl_hitm, rmt_hitm, tot_hitm),
> > macro FILTER_HITM is to filter out the cache line entries if its
> > overhead is less than 1%.
> >
> > This patch is to refactor macro FILTER_HITM. It uses more gernal name
> > FILTER_DISPLAY to replace the old name; and refines its parameter,
> > rather than passing field name for the data structure, it changes to
> > pass the cache line's statistic value and the sum value, this is more
> > flexsible, e.g. if consider to extend for sorting on all load hits
> > which combines multiple fields from structure c2c_stats.
>
> As it doesn't use field names anymore, I think it's better to change it to
> a static function.

Agreed. Will do.

Thanks,
Leo

> >
> > Signed-off-by: Leo Yan <[email protected]>
> > ---
> > tools/perf/builtin-c2c.c | 17 ++++++++++-------
> > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> > index 5cd30c083d6c..f11c3c84bb2b 100644
> > --- a/tools/perf/builtin-c2c.c
> > +++ b/tools/perf/builtin-c2c.c
> > @@ -2151,24 +2151,27 @@ static bool he__display(struct hist_entry *he, struct c2c_stats *stats)
> >
> > c2c_he = container_of(he, struct c2c_hist_entry, he);
> >
> > -#define FILTER_HITM(__h) \
> > - if (stats->__h) { \
> > - ld_dist = ((double)c2c_he->stats.__h / stats->__h); \
> > +#define FILTER_DISPLAY(val, sum) \
> > +{ \
> > + if ((sum)) { \
> > + ld_dist = ((double)(val) / (sum)); \
> > if (ld_dist < DISPLAY_LINE_LIMIT) \
> > he->filtered = HIST_FILTER__C2C; \
> > } else { \
> > he->filtered = HIST_FILTER__C2C; \
> > - }
> > + } \
> > +}
> >
> > switch (c2c.display) {
> > case DISPLAY_LCL:
> > - FILTER_HITM(lcl_hitm);
> > + FILTER_DISPLAY(c2c_he->stats.lcl_hitm, stats->lcl_hitm);
> > break;
> > case DISPLAY_RMT:
> > - FILTER_HITM(rmt_hitm);
> > + FILTER_DISPLAY(c2c_he->stats.rmt_hitm, stats->rmt_hitm);
> > break;
> > case DISPLAY_TOT:
> > - FILTER_HITM(tot_hitm);
> > + FILTER_DISPLAY(c2c_he->stats.tot_hitm, stats->tot_hitm);
> > + break;
> > default:
> > break;
> > }
> > --
> > 2.17.1
> >