[PATCH v1 3/8] perf stat: Display "none" for NaN with metric only json

Ian Rogers posted 8 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v1 3/8] perf stat: Display "none" for NaN with metric only json
Posted by Ian Rogers 1 month, 3 weeks ago
Return earlier for an empty unit case. If snprintf of the fmt doesn't
produce digits between vals and ends, as happens with NaN, make the
value "none" as happens in print_metric_end.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-display.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 5402998881c4..e392ee5efb45 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -609,19 +609,22 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
 {
 	struct outstate *os = ctx;
 	FILE *out = os->fh;
-	char buf[64], *vals, *ends;
+	char buf[64], *ends;
 	char tbuf[1024];
+	const char *vals;
 
 	if (!valid_only_metric(unit))
 		return;
 	unit = fixunit(tbuf, os->evsel, unit);
+	if (!unit[0])
+		return;
 	snprintf(buf, sizeof(buf), fmt ?: "", val);
-	ends = vals = skip_spaces(buf);
+	vals = ends = skip_spaces(buf);
 	while (isdigit(*ends) || *ends == '.')
 		ends++;
 	*ends = 0;
-	if (!unit[0] || !vals[0])
-		return;
+	if (!vals[0])
+		vals = "none";
 	fprintf(out, "%s\"%s\" : \"%s\"", os->first ? "" : ", ", unit, vals);
 	os->first = false;
 }
-- 
2.47.0.rc0.187.ge670bccf7e-goog
Re: [PATCH v1 3/8] perf stat: Display "none" for NaN with metric only json
Posted by Namhyung Kim 1 month, 2 weeks ago
On Fri, Oct 04, 2024 at 04:41:15PM -0700, Ian Rogers wrote:
> Return earlier for an empty unit case. If snprintf of the fmt doesn't
> produce digits between vals and ends, as happens with NaN, make the
> value "none" as happens in print_metric_end.

Then it could be "NaN" or is there any other case?  But probably "none"
would be more generic.

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/stat-display.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 5402998881c4..e392ee5efb45 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -609,19 +609,22 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
>  {
>  	struct outstate *os = ctx;
>  	FILE *out = os->fh;
> -	char buf[64], *vals, *ends;
> +	char buf[64], *ends;
>  	char tbuf[1024];
> +	const char *vals;
>  
>  	if (!valid_only_metric(unit))
>  		return;
>  	unit = fixunit(tbuf, os->evsel, unit);
> +	if (!unit[0])
> +		return;
>  	snprintf(buf, sizeof(buf), fmt ?: "", val);
> -	ends = vals = skip_spaces(buf);
> +	vals = ends = skip_spaces(buf);
>  	while (isdigit(*ends) || *ends == '.')
>  		ends++;
>  	*ends = 0;
> -	if (!unit[0] || !vals[0])
> -		return;
> +	if (!vals[0])
> +		vals = "none";
>  	fprintf(out, "%s\"%s\" : \"%s\"", os->first ? "" : ", ", unit, vals);
>  	os->first = false;
>  }
> -- 
> 2.47.0.rc0.187.ge670bccf7e-goog
>
Re: [PATCH v1 3/8] perf stat: Display "none" for NaN with metric only json
Posted by Ian Rogers 1 month, 2 weeks ago
On Tue, Oct 8, 2024 at 3:38 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Oct 04, 2024 at 04:41:15PM -0700, Ian Rogers wrote:
> > Return earlier for an empty unit case. If snprintf of the fmt doesn't
> > produce digits between vals and ends, as happens with NaN, make the
> > value "none" as happens in print_metric_end.
>
> Then it could be "NaN" or is there any other case?  But probably "none"
> would be more generic.

We use "none" here in print_metric_end as mentioned in the commit message:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat-display.c?h=perf-tools-next#n1090
If we used NaN then it'd be another case for users to parse, hence
trying to be consistent and using "none".

Thanks,
Ian

> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/stat-display.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 5402998881c4..e392ee5efb45 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -609,19 +609,22 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
> >  {
> >       struct outstate *os = ctx;
> >       FILE *out = os->fh;
> > -     char buf[64], *vals, *ends;
> > +     char buf[64], *ends;
> >       char tbuf[1024];
> > +     const char *vals;
> >
> >       if (!valid_only_metric(unit))
> >               return;
> >       unit = fixunit(tbuf, os->evsel, unit);
> > +     if (!unit[0])
> > +             return;
> >       snprintf(buf, sizeof(buf), fmt ?: "", val);
> > -     ends = vals = skip_spaces(buf);
> > +     vals = ends = skip_spaces(buf);
> >       while (isdigit(*ends) || *ends == '.')
> >               ends++;
> >       *ends = 0;
> > -     if (!unit[0] || !vals[0])
> > -             return;
> > +     if (!vals[0])
> > +             vals = "none";
> >       fprintf(out, "%s\"%s\" : \"%s\"", os->first ? "" : ", ", unit, vals);
> >       os->first = false;
> >  }
> > --
> > 2.47.0.rc0.187.ge670bccf7e-goog
> >