Re: [PATCH 09/10] perf tools: Compute other metrics

[Jiri Olsa <jolsa@redhat.com>]

On Mon, Jun 29, 2020 at 09:35:12AM -0700, Ian Rogers wrote:

SNIP

> 
> {
>  "BriefDescription": "All L2 hit counts",
>  "MetricExpr": "L2_RQSTS.DEMAND_DATA_RD_HIT + L2_RQSTS.PF_HIT +
> L2_RQSTS.RFO_HIT",
>  "MetricName": "DCache_L2_All_Hits",
> }
> {
>  "BriefDescription": "All L2 miss counts",
>  "MetricExpr": "MAX(L2_RQSTS.ALL_DEMAND_DATA_RD -
> L2_RQSTS.DEMAND_DATA_RD_HIT, 0) + L2_RQSTS.PF_MISS +
> L2_RQSTS.RFO_MISS",
>  "MetricName": "DCache_L2_All_Miss",
> }
> {
>  "BriefDescription": "All L2 counts",
>  "MetricExpr": "metric:DCache_L2_All_Hits + metric:DCache_L2_All_Miss",
>  "MetricName": "DCache_L2_All",
> }
> {
>  "BriefDescription": "DCache L2 hit rate",
>  "MetricExpr": "d_ratio(metric:DCache_L2_All_Hits, metric:DCache_L2_All)",
>  "MetricName": "DCache_L2_Hits",
>  "MetricGroup": "DCache_L2",
>  "ScaleUnit": "100%",
> },
> {
>  "BriefDescription": "DCache L2 miss rate",
>  "MetricExpr": "d_ratio(metric:DCache_L2_All_Miss, metric:DCache_L2_All)",
>  "MetricName": "DCache_L2_Misses",
>  "MetricGroup": "DCache_L2",
>  "ScaleUnit": "100%",
> },
> 
> Firstly, it should be clear that having this change makes the json far
> more readable! The current approach is to copy and paste resulting in
> 100s of characters wide expressions. This is a great improvement!
> 
> With these metrics the hope would be that 'perf stat -M DCache_L2 ...'
> is going to report just DCache_L2_Hits and DCache_L2_Misses. To
> compute these two metrics, as an example, DCache_L2_All_Hits is needed
> three times. My comment was meant to mean that it seems a little
> unfortunate to keep repeatedly evaluating the expression rather than
> to compute it once and reuse the result.

nice example, the code should evaluate the expression just once as long
as it's under same name.. I'll prepare new version and verify that's the
case with the example above

thanks,
jirka