diff mbox series

[v3,1/5] benchtests: Enable scripts/plot_strings.py to read stdin

Message ID 20210805074953.433483-1-naohirot@fujitsu.com
State Committed
Commit 3886eaff9d5a807732284a562f2d051e5d54fefa
Headers
Series benchtests: Add memset zero fill benchmark test |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Naohiro Tamura Aug. 5, 2021, 7:49 a.m. UTC 
  This patch enables scripts/plot_strings.py to read a benchmark result
file from stdin.
To keep backward compatibility, that is to keep accepting multiple of
benchmark result files in argument, blank argument doesn't mean stdin,
but '-' does.
Therefore nargs parameter of ArgumentParser.add_argument() method is
not changed to '?', but keep '+'.

ex:
  $ jq '.' bench-memset.out | plot_strings.py -
  $ jq '.' bench-memset.out | plot_strings.py - bench-memset-large.out
  $ plot_strings.py bench-memset.out bench-memset-large.out

error ex:
  $ jq '.' bench-memset.out | plot_strings.py
---
 benchtests/scripts/plot_strings.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Siddhesh Poyarekar Aug. 5, 2021, 7:56 a.m. UTC | #1 
On 8/5/21 1:19 PM, Naohiro Tamura via Libc-alpha wrote:
> This patch enables scripts/plot_strings.py to read a benchmark result
> file from stdin.
> To keep backward compatibility, that is to keep accepting multiple of
> benchmark result files in argument, blank argument doesn't mean stdin,
> but '-' does.
> Therefore nargs parameter of ArgumentParser.add_argument() method is
> not changed to '?', but keep '+'.
> 
> ex:
>    $ jq '.' bench-memset.out | plot_strings.py -
>    $ jq '.' bench-memset.out | plot_strings.py - bench-memset-large.out
>    $ plot_strings.py bench-memset.out bench-memset-large.out
> 
> error ex:
>    $ jq '.' bench-memset.out | plot_strings.py
> ---
>   benchtests/scripts/plot_strings.py | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)

Very nice!  LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> diff --git a/benchtests/scripts/plot_strings.py b/benchtests/scripts/plot_strings.py
> index c71f0804e4de..ec634692d9ad 100755
> --- a/benchtests/scripts/plot_strings.py
> +++ b/benchtests/scripts/plot_strings.py
> @@ -31,6 +31,7 @@ import json
>   import matplotlib as mpl
>   import numpy as np
>   import os
> +import sys
>   
>   try:
>       import jsonschema as validator
> @@ -331,8 +332,11 @@ def main(args):
>       for filename in args.bench:
>           bench = None
>   
> -        with open(filename, "r") as f:
> -            bench = json.load(f)
> +        if filename == '-':
> +            bench = json.load(sys.stdin)
> +        else:
> +            with open(filename, "r") as f:
> +                bench = json.load(f)
>   
>           validator.validate(bench, schema)
>   
> @@ -354,7 +358,8 @@ if __name__ == "__main__":
>   
>       # Required parameter
>       parser.add_argument("bench", nargs="+",
> -                        help="benchmark results file(s) in json format")
> +                        help="benchmark results file(s) in json format, " \
> +                        "and/or '-' as a benchmark result file from stdin")
>   
>       # Optional parameters
>       parser.add_argument("-b", "--baseline", type=str,
>
develop--- via Libc-alpha Sept. 8, 2021, 1:46 a.m. UTC | #2 
Hi Siddhesh, Thank you for the review comment.

Hi all, is there any other comment?
https://sourceware.org/pipermail/libc-alpha/2021-August/129838.html

Thanks.
Naohiro
> -----Original Message-----
> From: Siddhesh Poyarekar <siddhesh@gotplt.org>
> Sent: Thursday, August 5, 2021 4:57 PM
> To: Tamura, Naohiro/田村 直広 <naohirot@fujitsu.com>; libc-alpha@sourceware.org
> Subject: Re: [PATCH v3 1/5] benchtests: Enable scripts/plot_strings.py to read stdin
> 
> On 8/5/21 1:19 PM, Naohiro Tamura via Libc-alpha wrote:
> > This patch enables scripts/plot_strings.py to read a benchmark result
> > file from stdin.
> > To keep backward compatibility, that is to keep accepting multiple of
> > benchmark result files in argument, blank argument doesn't mean stdin,
> > but '-' does.
> > Therefore nargs parameter of ArgumentParser.add_argument() method is
> > not changed to '?', but keep '+'.
> >
> > ex:
> >    $ jq '.' bench-memset.out | plot_strings.py -
> >    $ jq '.' bench-memset.out | plot_strings.py - bench-memset-large.out
> >    $ plot_strings.py bench-memset.out bench-memset-large.out
> >
> > error ex:
> >    $ jq '.' bench-memset.out | plot_strings.py
> > ---
> >   benchtests/scripts/plot_strings.py | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> Very nice!  LGTM.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> 
> >
> > diff --git a/benchtests/scripts/plot_strings.py b/benchtests/scripts/plot_strings.py
> > index c71f0804e4de..ec634692d9ad 100755
> > --- a/benchtests/scripts/plot_strings.py
> > +++ b/benchtests/scripts/plot_strings.py
> > @@ -31,6 +31,7 @@ import json
> >   import matplotlib as mpl
> >   import numpy as np
> >   import os
> > +import sys
> >
> >   try:
> >       import jsonschema as validator
> > @@ -331,8 +332,11 @@ def main(args):
> >       for filename in args.bench:
> >           bench = None
> >
> > -        with open(filename, "r") as f:
> > -            bench = json.load(f)
> > +        if filename == '-':
> > +            bench = json.load(sys.stdin)
> > +        else:
> > +            with open(filename, "r") as f:
> > +                bench = json.load(f)
> >
> >           validator.validate(bench, schema)
> >
> > @@ -354,7 +358,8 @@ if __name__ == "__main__":
> >
> >       # Required parameter
> >       parser.add_argument("bench", nargs="+",
> > -                        help="benchmark results file(s) in json format")
> > +                        help="benchmark results file(s) in json format, " \
> > +                        "and/or '-' as a benchmark result file from stdin")
> >
> >       # Optional parameters
> >       parser.add_argument("-b", "--baseline", type=str,
> >
Siddhesh Poyarekar Sept. 8, 2021, 12:56 p.m. UTC | #3 
On 9/8/21 7:16 AM, naohirot@fujitsu.com wrote:
> Hi Siddhesh, Thank you for the review comment.
> 
> Hi all, is there any other comment?
> https://sourceware.org/pipermail/libc-alpha/2021-August/129838.html
> 

I approved the patch with this...

>>
>> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>

You may push this patch (you may still need review for other patches)  
right away if you want.

Siddhesh
develop--- via Libc-alpha Sept. 9, 2021, 12:22 a.m. UTC | #4 
Hi Siddhesh,

> From: Siddhesh Poyarekar <siddhesh@gotplt.org>
> Sent: Wednesday, September 8, 2021 9:56 PM
> 
> On 9/8/21 7:16 AM, naohirot@fujitsu.com wrote:
> > Hi Siddhesh, Thank you for the review comment.
> >
> > Hi all, is there any other comment?
> > https://sourceware.org/pipermail/libc-alpha/2021-August/129838.html
> >
> 
> I approved the patch with this...
> 
> >>
> >> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> >>

I see. Thanks!

> You may push this patch (you may still need review for other patches)
> right away if you want.

OK please merge it for me? I don't have the access right.
And could you review the other patches of this series too?
https://sourceware.org/pipermail/libc-alpha/2021-August/129841.html
https://sourceware.org/pipermail/libc-alpha/2021-August/129840.html
https://sourceware.org/pipermail/libc-alpha/2021-August/129839.html

Thanks.
Naohiro
Siddhesh Poyarekar Sept. 13, 2021, 3:45 a.m. UTC | #5 
On 9/9/21 5:52 AM, naohirot@fujitsu.com wrote:
> OK please merge it for me? I don't have the access right.

I've merged this and 3/5.

Siddhesh
diff mbox series

Patch

diff --git a/benchtests/scripts/plot_strings.py b/benchtests/scripts/plot_strings.py
index c71f0804e4de..ec634692d9ad 100755
--- a/benchtests/scripts/plot_strings.py
+++ b/benchtests/scripts/plot_strings.py
@@ -31,6 +31,7 @@  import json
 import matplotlib as mpl
 import numpy as np
 import os
+import sys
 
 try:
     import jsonschema as validator
@@ -331,8 +332,11 @@  def main(args):
     for filename in args.bench:
         bench = None
 
-        with open(filename, "r") as f:
-            bench = json.load(f)
+        if filename == '-':
+            bench = json.load(sys.stdin)
+        else:
+            with open(filename, "r") as f:
+                bench = json.load(f)
 
         validator.validate(bench, schema)
 
@@ -354,7 +358,8 @@  if __name__ == "__main__":
 
     # Required parameter
     parser.add_argument("bench", nargs="+",
-                        help="benchmark results file(s) in json format")
+                        help="benchmark results file(s) in json format, " \
+                        "and/or '-' as a benchmark result file from stdin")
 
     # Optional parameters
     parser.add_argument("-b", "--baseline", type=str,