* [PATCH v1 2/2] perf stat: Document --metric-no-threshold and threshold colors
2023-05-04 19:58 [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
@ 2023-05-04 19:58 ` Ian Rogers
2023-05-18 19:52 ` Liang, Kan
2023-05-15 16:03 ` [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2023-05-04 19:58 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Adrian Hunter, James Clark, Kan Liang, Andrii Nakryiko,
Eduard Zingerman, linux-perf-users, linux-kernel, Ahmad Yasin,
Stephane Eranian, Andi Kleen, Perry Taylor, Samantha Alt,
Caleb Biggers, Weilin Wang, Edward Baker
Cc: Ian Rogers
Document the threshold behavior for -M/--metrics.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/Documentation/perf-stat.txt | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 29bdcfa93f04..ff71399db738 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -353,6 +353,15 @@ small group that need not have multiplexing is lowered. This option
forbids the event merging logic from sharing events between groups and
may be used to increase accuracy in this case.
+--metric-no-threshold::
+Metric thresholds may increase the number of events necessary to
+compute whether a metric has exceeded its threshold expression. This
+may not be desirable, for example, as the events can introduce
+multiplexing. This option disables the adding of threshold expression
+events for a metric. However, if there are sufficient events to
+compute the threshold then the threshold is still computed and used to
+color the metric's computed value.
+
--quiet::
Don't print output, warnings or messages. This is useful with perf stat
record below to only write data to the perf.data file.
@@ -389,6 +398,12 @@ For a group all metrics from the group are added.
The events from the metrics are automatically measured.
See perf list output for the possible metrics and metricgroups.
+ When threshold information is available for a metric, the
+ color red is used to signify a metric has exceeded a threshold
+ while green shows it is hasn't. The default color means that
+ no threshold information was available or the threshold
+ couldn't be computed.
+
-A::
--no-aggr::
Do not aggregate counts across all monitored CPUs.
--
2.40.1.521.gf1e218fcd8-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v1 2/2] perf stat: Document --metric-no-threshold and threshold colors
2023-05-04 19:58 ` [PATCH v1 2/2] perf stat: Document --metric-no-threshold and threshold colors Ian Rogers
@ 2023-05-18 19:52 ` Liang, Kan
0 siblings, 0 replies; 8+ messages in thread
From: Liang, Kan @ 2023-05-18 19:52 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Adrian Hunter, James Clark, Andrii Nakryiko, Eduard Zingerman,
linux-perf-users, linux-kernel, Ahmad Yasin, Stephane Eranian,
Andi Kleen, Perry Taylor, Samantha Alt, Caleb Biggers,
Weilin Wang, Edward Baker
On 2023-05-04 3:58 p.m., Ian Rogers wrote:
> Document the threshold behavior for -M/--metrics.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/Documentation/perf-stat.txt | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 29bdcfa93f04..ff71399db738 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -353,6 +353,15 @@ small group that need not have multiplexing is lowered. This option
> forbids the event merging logic from sharing events between groups and
> may be used to increase accuracy in this case.
>
> +--metric-no-threshold::
> +Metric thresholds may increase the number of events necessary to
> +compute whether a metric has exceeded its threshold expression. This
> +may not be desirable, for example, as the events can introduce
> +multiplexing. This option disables the adding of threshold expression
> +events for a metric. However, if there are sufficient events to
> +compute the threshold then the threshold is still computed and used to
> +color the metric's computed value.
> +
> --quiet::
> Don't print output, warnings or messages. This is useful with perf stat
> record below to only write data to the perf.data file.
> @@ -389,6 +398,12 @@ For a group all metrics from the group are added.
> The events from the metrics are automatically measured.
> See perf list output for the possible metrics and metricgroups.
>
> + When threshold information is available for a metric, the
> + color red is used to signify a metric has exceeded a threshold
> + while green shows it is hasn't. The default color means that
A typo. "it is hasn't" -> "it hasn't"
Rest looks good to me.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> + no threshold information was available or the threshold
> + couldn't be computed.
> +
> -A::
> --no-aggr::
> Do not aggregate counts across all monitored CPUs.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy
2023-05-04 19:58 [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
2023-05-04 19:58 ` [PATCH v1 2/2] perf stat: Document --metric-no-threshold and threshold colors Ian Rogers
@ 2023-05-15 16:03 ` Ian Rogers
2023-05-16 7:40 ` Jiri Olsa
2023-05-18 19:47 ` Liang, Kan
3 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2023-05-15 16:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Adrian Hunter, James Clark, Kan Liang, Andrii Nakryiko,
Eduard Zingerman, linux-perf-users, linux-kernel, Ahmad Yasin,
Stephane Eranian, Andi Kleen, Perry Taylor, Samantha Alt,
Caleb Biggers, Weilin Wang, Edward Baker
On Thu, May 4, 2023 at 12:58 PM Ian Rogers <irogers@google.com> wrote:
>
> Currently the & and | operators are only used in metric thresholds
> like (from the tma_retiring metric):
> tma_retiring > 0.7 | tma_heavy_operations > 0.1
>
> Thresholds are always computed when present, but a lack events may
> mean the threshold can't be computed. This happens with the option
> --metric-no-threshold for say the metric tma_retiring on Tigerlake
> model CPUs. To fully compute the threshold tma_heavy_operations is
> needed and it needs the extra events of IDQ.MS_UOPS,
> UOPS_DECODED.DEC0, cpu/UOPS_DECODED.DEC0,cmask=1/ and
> IDQ.MITE_UOPS. So --metric-no-threshold is a useful option to reduce
> the number of events needed and potentially multiplexing of events.
>
> Rather than just fail threshold computations like this, we may know a
> result from just the left or right-hand side. So, for tma_retiring if
> its value is "> 0.7" we know it is over the threshold. This allows the
> metric to have the threshold coloring, when possible, without all the
> counters being programmed.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
Ping.
Thanks,
Ian
> ---
> tools/perf/tests/expr.c | 40 +++++++++++++++++++
> tools/perf/util/expr.y | 86 +++++++++++++++++++++++++++++++++--------
> 2 files changed, 109 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> index cbf0e0c74906..45c7fedb797a 100644
> --- a/tools/perf/tests/expr.c
> +++ b/tools/perf/tests/expr.c
> @@ -184,6 +184,46 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
> NULL, ctx) == 0);
> TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
>
> + /* The expression is a constant 0.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> /* Test toplogy constants appear well ordered. */
> expr__ctx_clear(ctx);
> TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0);
> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> index 250e444bf032..6b110f9f95c9 100644
> --- a/tools/perf/util/expr.y
> +++ b/tools/perf/util/expr.y
> @@ -123,20 +123,6 @@ static struct ids handle_id(struct expr_parse_ctx *ctx, char *id,
> * constant value using OP. Its invariant that there are no ids. If computing
> * ids for non-constants union the set of IDs that must be computed.
> */
> -#define BINARY_LONG_OP(RESULT, OP, LHS, RHS) \
> - if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> - assert(LHS.ids == NULL); \
> - assert(RHS.ids == NULL); \
> - if (isnan(LHS.val) || isnan(RHS.val)) { \
> - RESULT.val = NAN; \
> - } else { \
> - RESULT.val = (long)LHS.val OP (long)RHS.val; \
> - } \
> - RESULT.ids = NULL; \
> - } else { \
> - RESULT = union_expr(LHS, RHS); \
> - }
> -
> #define BINARY_OP(RESULT, OP, LHS, RHS) \
> if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> assert(LHS.ids == NULL); \
> @@ -213,9 +199,75 @@ expr: NUMBER
> }
> | ID { $$ = handle_id(ctx, $1, compute_ids, /*source_count=*/false); }
> | SOURCE_COUNT '(' ID ')' { $$ = handle_id(ctx, $3, compute_ids, /*source_count=*/true); }
> -| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> -| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> -| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> +| expr '|' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.ids = NULL;
> + $$.val = (fpclassify($1.val) == FP_ZERO && fpclassify($3.val) == FP_ZERO) ? 0 : 1;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) == FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) == FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '&' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) != FP_ZERO && fpclassify($3.val) != FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) != FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) != FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '^' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) == FP_ZERO) != (fpclassify($3.val) == FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> | expr '<' expr { BINARY_OP($$, <, $1, $3); }
> | expr '>' expr { BINARY_OP($$, >, $1, $3); }
> | expr '+' expr { BINARY_OP($$, +, $1, $3); }
> --
> 2.40.1.521.gf1e218fcd8-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy
2023-05-04 19:58 [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
2023-05-04 19:58 ` [PATCH v1 2/2] perf stat: Document --metric-no-threshold and threshold colors Ian Rogers
2023-05-15 16:03 ` [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
@ 2023-05-16 7:40 ` Jiri Olsa
2023-05-18 19:47 ` Liang, Kan
3 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2023-05-16 7:40 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
James Clark, Kan Liang, Andrii Nakryiko, Eduard Zingerman,
linux-perf-users, linux-kernel, Ahmad Yasin, Stephane Eranian,
Andi Kleen, Perry Taylor, Samantha Alt, Caleb Biggers,
Weilin Wang, Edward Baker
On Thu, May 04, 2023 at 12:58:02PM -0700, Ian Rogers wrote:
> Currently the & and | operators are only used in metric thresholds
> like (from the tma_retiring metric):
> tma_retiring > 0.7 | tma_heavy_operations > 0.1
>
> Thresholds are always computed when present, but a lack events may
> mean the threshold can't be computed. This happens with the option
> --metric-no-threshold for say the metric tma_retiring on Tigerlake
> model CPUs. To fully compute the threshold tma_heavy_operations is
> needed and it needs the extra events of IDQ.MS_UOPS,
> UOPS_DECODED.DEC0, cpu/UOPS_DECODED.DEC0,cmask=1/ and
> IDQ.MITE_UOPS. So --metric-no-threshold is a useful option to reduce
> the number of events needed and potentially multiplexing of events.
>
> Rather than just fail threshold computations like this, we may know a
> result from just the left or right-hand side. So, for tma_retiring if
> its value is "> 0.7" we know it is over the threshold. This allows the
> metric to have the threshold coloring, when possible, without all the
> counters being programmed.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
lgtm
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> tools/perf/tests/expr.c | 40 +++++++++++++++++++
> tools/perf/util/expr.y | 86 +++++++++++++++++++++++++++++++++--------
> 2 files changed, 109 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> index cbf0e0c74906..45c7fedb797a 100644
> --- a/tools/perf/tests/expr.c
> +++ b/tools/perf/tests/expr.c
> @@ -184,6 +184,46 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
> NULL, ctx) == 0);
> TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
>
> + /* The expression is a constant 0.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> /* Test toplogy constants appear well ordered. */
> expr__ctx_clear(ctx);
> TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0);
> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> index 250e444bf032..6b110f9f95c9 100644
> --- a/tools/perf/util/expr.y
> +++ b/tools/perf/util/expr.y
> @@ -123,20 +123,6 @@ static struct ids handle_id(struct expr_parse_ctx *ctx, char *id,
> * constant value using OP. Its invariant that there are no ids. If computing
> * ids for non-constants union the set of IDs that must be computed.
> */
> -#define BINARY_LONG_OP(RESULT, OP, LHS, RHS) \
> - if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> - assert(LHS.ids == NULL); \
> - assert(RHS.ids == NULL); \
> - if (isnan(LHS.val) || isnan(RHS.val)) { \
> - RESULT.val = NAN; \
> - } else { \
> - RESULT.val = (long)LHS.val OP (long)RHS.val; \
> - } \
> - RESULT.ids = NULL; \
> - } else { \
> - RESULT = union_expr(LHS, RHS); \
> - }
> -
> #define BINARY_OP(RESULT, OP, LHS, RHS) \
> if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> assert(LHS.ids == NULL); \
> @@ -213,9 +199,75 @@ expr: NUMBER
> }
> | ID { $$ = handle_id(ctx, $1, compute_ids, /*source_count=*/false); }
> | SOURCE_COUNT '(' ID ')' { $$ = handle_id(ctx, $3, compute_ids, /*source_count=*/true); }
> -| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> -| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> -| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> +| expr '|' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.ids = NULL;
> + $$.val = (fpclassify($1.val) == FP_ZERO && fpclassify($3.val) == FP_ZERO) ? 0 : 1;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) == FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) == FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '&' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) != FP_ZERO && fpclassify($3.val) != FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) != FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) != FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '^' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) == FP_ZERO) != (fpclassify($3.val) == FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> | expr '<' expr { BINARY_OP($$, <, $1, $3); }
> | expr '>' expr { BINARY_OP($$, >, $1, $3); }
> | expr '+' expr { BINARY_OP($$, +, $1, $3); }
> --
> 2.40.1.521.gf1e218fcd8-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy
2023-05-04 19:58 [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy Ian Rogers
` (2 preceding siblings ...)
2023-05-16 7:40 ` Jiri Olsa
@ 2023-05-18 19:47 ` Liang, Kan
2023-06-05 17:32 ` Ian Rogers
3 siblings, 1 reply; 8+ messages in thread
From: Liang, Kan @ 2023-05-18 19:47 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Adrian Hunter, James Clark, Andrii Nakryiko, Eduard Zingerman,
linux-perf-users, linux-kernel, Ahmad Yasin, Stephane Eranian,
Andi Kleen, Perry Taylor, Samantha Alt, Caleb Biggers,
Weilin Wang, Edward Baker
On 2023-05-04 3:58 p.m., Ian Rogers wrote:
> Currently the & and | operators are only used in metric thresholds
> like (from the tma_retiring metric):
> tma_retiring > 0.7 | tma_heavy_operations > 0.1
>
> Thresholds are always computed when present, but a lack events may
> mean the threshold can't be computed. This happens with the option
> --metric-no-threshold for say the metric tma_retiring on Tigerlake
> model CPUs. To fully compute the threshold tma_heavy_operations is
> needed and it needs the extra events of IDQ.MS_UOPS,
> UOPS_DECODED.DEC0, cpu/UOPS_DECODED.DEC0,cmask=1/ and
> IDQ.MITE_UOPS. So --metric-no-threshold is a useful option to reduce
> the number of events needed and potentially multiplexing of events.
>
> Rather than just fail threshold computations like this, we may know a
> result from just the left or right-hand side. So, for tma_retiring if
> its value is "> 0.7" we know it is over the threshold. This allows the
> metric to have the threshold coloring, when possible, without all the
> counters being programmed.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
The patch works well on my machine.
Tested-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> tools/perf/tests/expr.c | 40 +++++++++++++++++++
> tools/perf/util/expr.y | 86 +++++++++++++++++++++++++++++++++--------
> 2 files changed, 109 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> index cbf0e0c74906..45c7fedb797a 100644
> --- a/tools/perf/tests/expr.c
> +++ b/tools/perf/tests/expr.c
> @@ -184,6 +184,46 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
> NULL, ctx) == 0);
> TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
>
> + /* The expression is a constant 0.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 & EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 & 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("1 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 1", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("0 | EVENT1 > 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> + expr__ctx_clear(ctx);
> + TEST_ASSERT_VAL("find ids",
> + expr__find_ids("EVENT1 > 0 | 0", NULL, ctx) == 0);
> + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> +
> /* Test toplogy constants appear well ordered. */
> expr__ctx_clear(ctx);
> TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0);
> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> index 250e444bf032..6b110f9f95c9 100644
> --- a/tools/perf/util/expr.y
> +++ b/tools/perf/util/expr.y
> @@ -123,20 +123,6 @@ static struct ids handle_id(struct expr_parse_ctx *ctx, char *id,
> * constant value using OP. Its invariant that there are no ids. If computing
> * ids for non-constants union the set of IDs that must be computed.
> */
> -#define BINARY_LONG_OP(RESULT, OP, LHS, RHS) \
> - if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> - assert(LHS.ids == NULL); \
> - assert(RHS.ids == NULL); \
> - if (isnan(LHS.val) || isnan(RHS.val)) { \
> - RESULT.val = NAN; \
> - } else { \
> - RESULT.val = (long)LHS.val OP (long)RHS.val; \
> - } \
> - RESULT.ids = NULL; \
> - } else { \
> - RESULT = union_expr(LHS, RHS); \
> - }
> -
> #define BINARY_OP(RESULT, OP, LHS, RHS) \
> if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> assert(LHS.ids == NULL); \
> @@ -213,9 +199,75 @@ expr: NUMBER
> }
> | ID { $$ = handle_id(ctx, $1, compute_ids, /*source_count=*/false); }
> | SOURCE_COUNT '(' ID ')' { $$ = handle_id(ctx, $3, compute_ids, /*source_count=*/true); }
> -| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> -| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> -| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> +| expr '|' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.ids = NULL;
> + $$.val = (fpclassify($1.val) == FP_ZERO && fpclassify($3.val) == FP_ZERO) ? 0 : 1;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) == FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) == FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 1;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '&' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) != FP_ZERO && fpclassify($3.val) != FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else if (is_const($1.val)) {
> + assert($1.ids == NULL);
> + if (fpclassify($1.val) != FP_ZERO) {
> + $$ = $3;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($3.ids);
> + }
> + } else if (is_const($3.val)) {
> + assert($3.ids == NULL);
> + if (fpclassify($3.val) != FP_ZERO) {
> + $$ = $1;
> + } else {
> + $$.val = 0;
> + $$.ids = NULL;
> + ids__free($1.ids);
> + }
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> +| expr '^' expr
> +{
> + if (is_const($1.val) && is_const($3.val)) {
> + assert($1.ids == NULL);
> + assert($3.ids == NULL);
> + $$.val = (fpclassify($1.val) == FP_ZERO) != (fpclassify($3.val) == FP_ZERO) ? 1 : 0;
> + $$.ids = NULL;
> + } else {
> + $$ = union_expr($1, $3);
> + }
> +}
> | expr '<' expr { BINARY_OP($$, <, $1, $3); }
> | expr '>' expr { BINARY_OP($$, >, $1, $3); }
> | expr '+' expr { BINARY_OP($$, +, $1, $3); }
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy
2023-05-18 19:47 ` Liang, Kan
@ 2023-06-05 17:32 ` Ian Rogers
2023-06-05 19:30 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2023-06-05 17:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Adrian Hunter, James Clark,
Andrii Nakryiko, Eduard Zingerman, linux-perf-users, linux-kernel,
Ahmad Yasin, Stephane Eranian, Andi Kleen, Perry Taylor,
Samantha Alt, Caleb Biggers, Weilin Wang, Edward Baker,
Liang, Kan
On Thu, May 18, 2023 at 12:47 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2023-05-04 3:58 p.m., Ian Rogers wrote:
> > Currently the & and | operators are only used in metric thresholds
> > like (from the tma_retiring metric):
> > tma_retiring > 0.7 | tma_heavy_operations > 0.1
> >
> > Thresholds are always computed when present, but a lack events may
> > mean the threshold can't be computed. This happens with the option
> > --metric-no-threshold for say the metric tma_retiring on Tigerlake
> > model CPUs. To fully compute the threshold tma_heavy_operations is
> > needed and it needs the extra events of IDQ.MS_UOPS,
> > UOPS_DECODED.DEC0, cpu/UOPS_DECODED.DEC0,cmask=1/ and
> > IDQ.MITE_UOPS. So --metric-no-threshold is a useful option to reduce
> > the number of events needed and potentially multiplexing of events.
> >
> > Rather than just fail threshold computations like this, we may know a
> > result from just the left or right-hand side. So, for tma_retiring if
> > its value is "> 0.7" we know it is over the threshold. This allows the
> > metric to have the threshold coloring, when possible, without all the
> > counters being programmed.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
>
> The patch works well on my machine.
>
> Tested-by: Kan Liang <kan.liang@linux.intel.com>
>
> Thanks,
> Kan
Arnaldo, could we take this set?
Thanks,
Ian
> > tools/perf/tests/expr.c | 40 +++++++++++++++++++
> > tools/perf/util/expr.y | 86 +++++++++++++++++++++++++++++++++--------
> > 2 files changed, 109 insertions(+), 17 deletions(-)
> >
> > diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> > index cbf0e0c74906..45c7fedb797a 100644
> > --- a/tools/perf/tests/expr.c
> > +++ b/tools/perf/tests/expr.c
> > @@ -184,6 +184,46 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
> > NULL, ctx) == 0);
> > TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> >
> > + /* The expression is a constant 0.0 without needing to evaluate EVENT1. */
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("0 & EVENT1 > 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("EVENT1 > 0 & 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("1 & EVENT1 > 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("EVENT1 > 0 & 1", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > +
> > + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("1 | EVENT1 > 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("EVENT1 > 0 | 1", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("0 | EVENT1 > 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > + expr__ctx_clear(ctx);
> > + TEST_ASSERT_VAL("find ids",
> > + expr__find_ids("EVENT1 > 0 | 0", NULL, ctx) == 0);
> > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > +
> > /* Test toplogy constants appear well ordered. */
> > expr__ctx_clear(ctx);
> > TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0);
> > diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> > index 250e444bf032..6b110f9f95c9 100644
> > --- a/tools/perf/util/expr.y
> > +++ b/tools/perf/util/expr.y
> > @@ -123,20 +123,6 @@ static struct ids handle_id(struct expr_parse_ctx *ctx, char *id,
> > * constant value using OP. Its invariant that there are no ids. If computing
> > * ids for non-constants union the set of IDs that must be computed.
> > */
> > -#define BINARY_LONG_OP(RESULT, OP, LHS, RHS) \
> > - if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> > - assert(LHS.ids == NULL); \
> > - assert(RHS.ids == NULL); \
> > - if (isnan(LHS.val) || isnan(RHS.val)) { \
> > - RESULT.val = NAN; \
> > - } else { \
> > - RESULT.val = (long)LHS.val OP (long)RHS.val; \
> > - } \
> > - RESULT.ids = NULL; \
> > - } else { \
> > - RESULT = union_expr(LHS, RHS); \
> > - }
> > -
> > #define BINARY_OP(RESULT, OP, LHS, RHS) \
> > if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> > assert(LHS.ids == NULL); \
> > @@ -213,9 +199,75 @@ expr: NUMBER
> > }
> > | ID { $$ = handle_id(ctx, $1, compute_ids, /*source_count=*/false); }
> > | SOURCE_COUNT '(' ID ')' { $$ = handle_id(ctx, $3, compute_ids, /*source_count=*/true); }
> > -| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> > -| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> > -| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> > +| expr '|' expr
> > +{
> > + if (is_const($1.val) && is_const($3.val)) {
> > + assert($1.ids == NULL);
> > + assert($3.ids == NULL);
> > + $$.ids = NULL;
> > + $$.val = (fpclassify($1.val) == FP_ZERO && fpclassify($3.val) == FP_ZERO) ? 0 : 1;
> > + } else if (is_const($1.val)) {
> > + assert($1.ids == NULL);
> > + if (fpclassify($1.val) == FP_ZERO) {
> > + $$ = $3;
> > + } else {
> > + $$.val = 1;
> > + $$.ids = NULL;
> > + ids__free($3.ids);
> > + }
> > + } else if (is_const($3.val)) {
> > + assert($3.ids == NULL);
> > + if (fpclassify($3.val) == FP_ZERO) {
> > + $$ = $1;
> > + } else {
> > + $$.val = 1;
> > + $$.ids = NULL;
> > + ids__free($1.ids);
> > + }
> > + } else {
> > + $$ = union_expr($1, $3);
> > + }
> > +}
> > +| expr '&' expr
> > +{
> > + if (is_const($1.val) && is_const($3.val)) {
> > + assert($1.ids == NULL);
> > + assert($3.ids == NULL);
> > + $$.val = (fpclassify($1.val) != FP_ZERO && fpclassify($3.val) != FP_ZERO) ? 1 : 0;
> > + $$.ids = NULL;
> > + } else if (is_const($1.val)) {
> > + assert($1.ids == NULL);
> > + if (fpclassify($1.val) != FP_ZERO) {
> > + $$ = $3;
> > + } else {
> > + $$.val = 0;
> > + $$.ids = NULL;
> > + ids__free($3.ids);
> > + }
> > + } else if (is_const($3.val)) {
> > + assert($3.ids == NULL);
> > + if (fpclassify($3.val) != FP_ZERO) {
> > + $$ = $1;
> > + } else {
> > + $$.val = 0;
> > + $$.ids = NULL;
> > + ids__free($1.ids);
> > + }
> > + } else {
> > + $$ = union_expr($1, $3);
> > + }
> > +}
> > +| expr '^' expr
> > +{
> > + if (is_const($1.val) && is_const($3.val)) {
> > + assert($1.ids == NULL);
> > + assert($3.ids == NULL);
> > + $$.val = (fpclassify($1.val) == FP_ZERO) != (fpclassify($3.val) == FP_ZERO) ? 1 : 0;
> > + $$.ids = NULL;
> > + } else {
> > + $$ = union_expr($1, $3);
> > + }
> > +}
> > | expr '<' expr { BINARY_OP($$, <, $1, $3); }
> > | expr '>' expr { BINARY_OP($$, >, $1, $3); }
> > | expr '+' expr { BINARY_OP($$, +, $1, $3); }
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/2] perf expr: Make the evaluation of & and | logical and lazy
2023-06-05 17:32 ` Ian Rogers
@ 2023-06-05 19:30 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-06-05 19:30 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Adrian Hunter, James Clark,
Andrii Nakryiko, Eduard Zingerman, linux-perf-users, linux-kernel,
Ahmad Yasin, Stephane Eranian, Andi Kleen, Perry Taylor,
Samantha Alt, Caleb Biggers, Weilin Wang, Edward Baker,
Liang, Kan
Em Mon, Jun 05, 2023 at 10:32:18AM -0700, Ian Rogers escreveu:
> On Thu, May 18, 2023 at 12:47 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >
> >
> >
> > On 2023-05-04 3:58 p.m., Ian Rogers wrote:
> > > Currently the & and | operators are only used in metric thresholds
> > > like (from the tma_retiring metric):
> > > tma_retiring > 0.7 | tma_heavy_operations > 0.1
> > >
> > > Thresholds are always computed when present, but a lack events may
> > > mean the threshold can't be computed. This happens with the option
> > > --metric-no-threshold for say the metric tma_retiring on Tigerlake
> > > model CPUs. To fully compute the threshold tma_heavy_operations is
> > > needed and it needs the extra events of IDQ.MS_UOPS,
> > > UOPS_DECODED.DEC0, cpu/UOPS_DECODED.DEC0,cmask=1/ and
> > > IDQ.MITE_UOPS. So --metric-no-threshold is a useful option to reduce
> > > the number of events needed and potentially multiplexing of events.
> > >
> > > Rather than just fail threshold computations like this, we may know a
> > > result from just the left or right-hand side. So, for tma_retiring if
> > > its value is "> 0.7" we know it is over the threshold. This allows the
> > > metric to have the threshold coloring, when possible, without all the
> > > counters being programmed.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> >
> > The patch works well on my machine.
> >
> > Tested-by: Kan Liang <kan.liang@linux.intel.com>
> >
> > Thanks,
> > Kan
>
> Arnaldo, could we take this set?
Thanks, applied.
- Arnaldo
> Thanks,
> Ian
>
> > > tools/perf/tests/expr.c | 40 +++++++++++++++++++
> > > tools/perf/util/expr.y | 86 +++++++++++++++++++++++++++++++++--------
> > > 2 files changed, 109 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> > > index cbf0e0c74906..45c7fedb797a 100644
> > > --- a/tools/perf/tests/expr.c
> > > +++ b/tools/perf/tests/expr.c
> > > @@ -184,6 +184,46 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
> > > NULL, ctx) == 0);
> > > TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > >
> > > + /* The expression is a constant 0.0 without needing to evaluate EVENT1. */
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("0 & EVENT1 > 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("EVENT1 > 0 & 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("1 & EVENT1 > 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("EVENT1 > 0 & 1", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > > +
> > > + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("1 | EVENT1 > 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("EVENT1 > 0 | 1", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("0 | EVENT1 > 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > > + expr__ctx_clear(ctx);
> > > + TEST_ASSERT_VAL("find ids",
> > > + expr__find_ids("EVENT1 > 0 | 0", NULL, ctx) == 0);
> > > + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
> > > + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1", &val_ptr));
> > > +
> > > /* Test toplogy constants appear well ordered. */
> > > expr__ctx_clear(ctx);
> > > TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0);
> > > diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> > > index 250e444bf032..6b110f9f95c9 100644
> > > --- a/tools/perf/util/expr.y
> > > +++ b/tools/perf/util/expr.y
> > > @@ -123,20 +123,6 @@ static struct ids handle_id(struct expr_parse_ctx *ctx, char *id,
> > > * constant value using OP. Its invariant that there are no ids. If computing
> > > * ids for non-constants union the set of IDs that must be computed.
> > > */
> > > -#define BINARY_LONG_OP(RESULT, OP, LHS, RHS) \
> > > - if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> > > - assert(LHS.ids == NULL); \
> > > - assert(RHS.ids == NULL); \
> > > - if (isnan(LHS.val) || isnan(RHS.val)) { \
> > > - RESULT.val = NAN; \
> > > - } else { \
> > > - RESULT.val = (long)LHS.val OP (long)RHS.val; \
> > > - } \
> > > - RESULT.ids = NULL; \
> > > - } else { \
> > > - RESULT = union_expr(LHS, RHS); \
> > > - }
> > > -
> > > #define BINARY_OP(RESULT, OP, LHS, RHS) \
> > > if (!compute_ids || (is_const(LHS.val) && is_const(RHS.val))) { \
> > > assert(LHS.ids == NULL); \
> > > @@ -213,9 +199,75 @@ expr: NUMBER
> > > }
> > > | ID { $$ = handle_id(ctx, $1, compute_ids, /*source_count=*/false); }
> > > | SOURCE_COUNT '(' ID ')' { $$ = handle_id(ctx, $3, compute_ids, /*source_count=*/true); }
> > > -| expr '|' expr { BINARY_LONG_OP($$, |, $1, $3); }
> > > -| expr '&' expr { BINARY_LONG_OP($$, &, $1, $3); }
> > > -| expr '^' expr { BINARY_LONG_OP($$, ^, $1, $3); }
> > > +| expr '|' expr
> > > +{
> > > + if (is_const($1.val) && is_const($3.val)) {
> > > + assert($1.ids == NULL);
> > > + assert($3.ids == NULL);
> > > + $$.ids = NULL;
> > > + $$.val = (fpclassify($1.val) == FP_ZERO && fpclassify($3.val) == FP_ZERO) ? 0 : 1;
> > > + } else if (is_const($1.val)) {
> > > + assert($1.ids == NULL);
> > > + if (fpclassify($1.val) == FP_ZERO) {
> > > + $$ = $3;
> > > + } else {
> > > + $$.val = 1;
> > > + $$.ids = NULL;
> > > + ids__free($3.ids);
> > > + }
> > > + } else if (is_const($3.val)) {
> > > + assert($3.ids == NULL);
> > > + if (fpclassify($3.val) == FP_ZERO) {
> > > + $$ = $1;
> > > + } else {
> > > + $$.val = 1;
> > > + $$.ids = NULL;
> > > + ids__free($1.ids);
> > > + }
> > > + } else {
> > > + $$ = union_expr($1, $3);
> > > + }
> > > +}
> > > +| expr '&' expr
> > > +{
> > > + if (is_const($1.val) && is_const($3.val)) {
> > > + assert($1.ids == NULL);
> > > + assert($3.ids == NULL);
> > > + $$.val = (fpclassify($1.val) != FP_ZERO && fpclassify($3.val) != FP_ZERO) ? 1 : 0;
> > > + $$.ids = NULL;
> > > + } else if (is_const($1.val)) {
> > > + assert($1.ids == NULL);
> > > + if (fpclassify($1.val) != FP_ZERO) {
> > > + $$ = $3;
> > > + } else {
> > > + $$.val = 0;
> > > + $$.ids = NULL;
> > > + ids__free($3.ids);
> > > + }
> > > + } else if (is_const($3.val)) {
> > > + assert($3.ids == NULL);
> > > + if (fpclassify($3.val) != FP_ZERO) {
> > > + $$ = $1;
> > > + } else {
> > > + $$.val = 0;
> > > + $$.ids = NULL;
> > > + ids__free($1.ids);
> > > + }
> > > + } else {
> > > + $$ = union_expr($1, $3);
> > > + }
> > > +}
> > > +| expr '^' expr
> > > +{
> > > + if (is_const($1.val) && is_const($3.val)) {
> > > + assert($1.ids == NULL);
> > > + assert($3.ids == NULL);
> > > + $$.val = (fpclassify($1.val) == FP_ZERO) != (fpclassify($3.val) == FP_ZERO) ? 1 : 0;
> > > + $$.ids = NULL;
> > > + } else {
> > > + $$ = union_expr($1, $3);
> > > + }
> > > +}
> > > | expr '<' expr { BINARY_OP($$, <, $1, $3); }
> > > | expr '>' expr { BINARY_OP($$, >, $1, $3); }
> > > | expr '+' expr { BINARY_OP($$, +, $1, $3); }
--
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread