Re: [Fuego] [PATCH] Dhrystone: Add 'one run through Dhrystone' value in the report

See comments inline below.

> -----Original Message-----
> From: [email protected] <[email protected]=
m>
>=20
> From: sireesha <[email protected]>
>=20
> parser.py: update parser file to parse 'Microseconds of one run through D=
hrystone'
Please keep lines in the commit message less than 80 chars long.  The one a=
bove is
a bit too long.

>=20
> reference.json: Add reference for 'Microseconds of one run through Dhryst=
one'
>=20
> Signed-off-by: sireesha <[email protected]>
> ---
>  tests/Benchmark.Dhrystone/parser.py      | 6 +++---
>  tests/Benchmark.Dhrystone/reference.json | 5 +++++
>  2 files changed, 8 insertions(+), 3 deletions(-)
>=20
> diff --git a/tests/Benchmark.Dhrystone/parser.py b/tests/Benchmark.Dhryst=
one/parser.py
> index 261d661..62447e9 100755
> --- a/tests/Benchmark.Dhrystone/parser.py
> +++ b/tests/Benchmark.Dhrystone/parser.py
> @@ -3,12 +3,12 @@
>  import os, re, sys
>  import common as plib
>=20
> -regex_string =3D "^(Dhrystones.per.Second:)(\ *)([\d]{1,8}.?[\d]{1,3})(.=
*)$"
> -
> +regex_string =3D "^(Microseconds.for.one.run.through.Dhrystone:|Dhryston=
es.per.Second:)(\ *)([\d]{1,8}.?[\d]{1,3})(.*)$"

I'm don't like using periods where spaces are trying to be matched, but
your addition is no worse than what was here before.

>  measurements =3D {}
> +
>  matches =3D plib.parse_log(regex_string)
>=20
>  if matches:
> -    measurements['default.Dhrystone'] =3D [{"name": "Score", "measure" :=
 float(matches[0][2])}]
> +    measurements['default.Dhrystone'] =3D [{"name": "microseconds_onerun=
", "measure" : float(matches[0][2])},{"name": "Score",

I kind of prefer 'onerun_microseconds', but that's just me.  I can live wit=
h this, since it matches
the order of the words in the testlog.txt file.  You might want to consider=
 'microseconds_for_one_run'.
It's just aesthetics, though.

> "measure" : float(matches[1][2])}]
>=20
>  sys.exit(plib.process(measurements))
> diff --git a/tests/Benchmark.Dhrystone/reference.json b/tests/Benchmark.D=
hrystone/reference.json
> index 532ac16..443ebcf 100644
> --- a/tests/Benchmark.Dhrystone/reference.json
> +++ b/tests/Benchmark.Dhrystone/reference.json
> @@ -9,7 +9,12 @@
>                          {
>                              "name":"Score",
>                              "unit":"Dhrystones/s"
> +                        },
> +			{
> +                            "name":"microseconds_onerun",
> +                            "unit":"us"
>                          }
> +
>                      ]
>                  }
>              ]
> --
> 2.20.1
>=20

Looks good.  You haven't updated chart_config.json to show this new measure=
.
Did you want to do that?

I've applied and pushed this.  Please send any new patches based on this
one being already applied.

Thanks,
 -- Tim