[PATCH 0/3] benchtests: Increase interoperability and robustness

[PATCH 0/3] benchtests: Increase interoperability and robustness

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

While benchmarking and reporting numbers through the compare_script.py script,
I found the latter output table not so useful as input of other tools. To make the
output simpler, alias raw data, two options are suggested (--no-diff and --no-header).
Also bad input can come from user, so catch these and quit properly.

Leonardo Sandoval (3):
  benchtests: Add --no-diff option to avoid diff calculation
  benchtests: Add --no-header option to omit header
  benchtests: Catch exceptions in input arguments

 ChangeLog                             | 16 ++++++++++
 benchtests/scripts/compare_strings.py | 44 +++++++++++++++++++--------
 2 files changed, 48 insertions(+), 12 deletions(-)

-- 
2.17.0

[PATCH 3/3] benchtests: Catch exceptions in input arguments

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Catch runtime exceptions in case the user provided either wrong base
function or attribute(s). In any of the latter, quit immediately with
non-zero return code.

	* benchtests/scripts/compare_string.py: (process_results) Catch
	exception in non-existent base_func.
	(process_results) Catch exception in non-existent attribute.
---
 ChangeLog                             |  6 ++++++
 benchtests/scripts/compare_strings.py | 16 ++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index cc3e50e3b8d..20d55a47759 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
+
+	* benchtests/scripts/compare_string.py:	(process_results) Catch
+	exception in non-existent base_func.
+	(process_results) Catch exception in non-existent attribute
+
 2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
 
 	* benchtests/scripts/compare_string.py: Add --no-header	option
diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
index c629bba77fa..077b53fa6c9 100755
--- a/benchtests/scripts/compare_strings.py
+++ b/benchtests/scripts/compare_strings.py
@@ -93,7 +93,12 @@ def process_results(results, attrs, base_func, graph, no_diff, no_header):
 
         base_index = 0
         if base_func:
-            base_index = results['functions'][f]['ifuncs'].index(base_func)
+            try:
+                base_index = results['functions'][f]['ifuncs'].index(base_func)
+            except ValueError as ve:
+                sys.stderr.write('Invalid -b "%s" parameter. Options: %s.\n' % (base_func,
+                                                                          ', '.join(results['functions'][f]['ifuncs'])))
+                sys.exit(os.EX_DATAERR)
 
         if not no_header:
             print('Function: %s' % f)
@@ -103,7 +108,12 @@ def process_results(results, attrs, base_func, graph, no_diff, no_header):
 
         graph_res = {}
         for res in results['functions'][f]['results']:
-            attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
+            try:
+                attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
+            except KeyError as ke:
+                sys.stderr.write('Invalid -a %s parameter. Options: %s.\n' % (ke,
+                                                                          ', '.join([a for a in res.keys() if a != 'timings'])))
+                sys.exit(os.EX_DATAERR)
             i = 0
             key = ', '.join(attr_list)
             sys.stdout.write('%36s: ' % key)
@@ -163,3 +173,5 @@ if __name__ == '__main__':
 
     args = parser.parse_args()
     main(args)
+
+    return os.EX_OK
-- 
2.17.0

Re: [PATCH 3/3] benchtests: Catch exceptions in input arguments

[Leonardo Sandoval]

please ignore this patch (the rest of the series is fine). Will send a
V2.

Leo

On Fri, 2018-05-25 at 10:58 -0500,
leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Catch runtime exceptions in case the user provided either wrong base
> function or attribute(s). In any of the latter, quit immediately with
> non-zero return code.
> 
> 	* benchtests/scripts/compare_string.py: (process_results) Catch
> 	exception in non-existent base_func.
> 	(process_results) Catch exception in non-existent attribute.
> ---
>  ChangeLog                             |  6 ++++++
>  benchtests/scripts/compare_strings.py | 16 ++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index cc3e50e3b8d..20d55a47759 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com
> >
> +
> +	* benchtests/scripts/compare_string.py:	(process_resu
> lts) Catch
> +	exception in non-existent base_func.
> +	(process_results) Catch exception in non-existent attribute
> +
>  2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com
> >
>  
>  	* benchtests/scripts/compare_string.py: Add --no-header	
> option
> diff --git a/benchtests/scripts/compare_strings.py
> b/benchtests/scripts/compare_strings.py
> index c629bba77fa..077b53fa6c9 100755
> --- a/benchtests/scripts/compare_strings.py
> +++ b/benchtests/scripts/compare_strings.py
> @@ -93,7 +93,12 @@ def process_results(results, attrs, base_func,
> graph, no_diff, no_header):
>  
>          base_index = 0
>          if base_func:
> -            base_index =
> results['functions'][f]['ifuncs'].index(base_func)
> +            try:
> +                base_index =
> results['functions'][f]['ifuncs'].index(base_func)
> +            except ValueError as ve:
> +                sys.stderr.write('Invalid -b "%s" parameter.
> Options: %s.\n' % (base_func,
> +                                                                    
>       ', '.join(results['functions'][f]['ifuncs'])))
> +                sys.exit(os.EX_DATAERR)
>  
>          if not no_header:
>              print('Function: %s' % f)
> @@ -103,7 +108,12 @@ def process_results(results, attrs, base_func,
> graph, no_diff, no_header):
>  
>          graph_res = {}
>          for res in results['functions'][f]['results']:
> -            attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
> +            try:
> +                attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
> +            except KeyError as ke:
> +                sys.stderr.write('Invalid -a %s parameter. Options:
> %s.\n' % (ke,
> +                                                                    
>       ', '.join([a for a in res.keys() if a != 'timings'])))
> +                sys.exit(os.EX_DATAERR)
>              i = 0
>              key = ', '.join(attr_list)
>              sys.stdout.write('%36s: ' % key)
> @@ -163,3 +173,5 @@ if __name__ == '__main__':
>  
>      args = parser.parse_args()
>      main(args)
> +
> +    return os.EX_OK

[PATCH 1/3] benchtests: Add --no-diff option to avoid diff calculation

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Having the report with no diff numbers (percentages numbers
enclosed within parenthesis) makes column separation cleaner
(just spaces), allowing easier interaction with other tools.

	* benchtests/scripts/compare_string.py: Add --no-diff
	option to avoid diff calculation.
---
 ChangeLog                             |  5 +++++
 benchtests/scripts/compare_strings.py | 15 +++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c7436c4f333..afddd77315a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
+
+	* benchtests/scripts/compare_string.py: Add --no-diff option
+	to avoid diff calculation.
+
 2018-05-24  Tulio Magno Quites Machado Filho  <tuliom@linux.ibm.com>
 	    Gabriel F. T. Gomes  <gabriel@inconstante.eti.br>
 
diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
index d37442076bf..ceeccea13b6 100755
--- a/benchtests/scripts/compare_strings.py
+++ b/benchtests/scripts/compare_strings.py
@@ -79,7 +79,7 @@ def draw_graph(f, v, ifuncs, results):
     pylab.savefig('%s-%s.png' % (f, v), bbox_inches='tight')
 
 
-def process_results(results, attrs, base_func, graph):
+def process_results(results, attrs, base_func, graph, no_diff):
     """ Process results and print them
 
     Args:
@@ -107,10 +107,11 @@ def process_results(results, attrs, base_func, graph):
             graph_res[key] = res['timings']
             for t in res['timings']:
                 sys.stdout.write ('%12.2f' % t)
-                if i != base_index:
-                    base = res['timings'][base_index]
-                    diff = (base - t) * 100 / base
-                    sys.stdout.write (' (%6.2f%%)' % diff)
+                if not no_diff:
+                    if i != base_index:
+                        base = res['timings'][base_index]
+                        diff = (base - t) * 100 / base
+                        sys.stdout.write (' (%6.2f%%)' % diff)
                 sys.stdout.write('\t')
                 i = i + 1
             print('')
@@ -132,7 +133,7 @@ def main(args):
     attrs = args.attributes.split(',')
 
     results = parse_file(args.input, args.schema)
-    process_results(results, attrs, base_func, args.graph)
+    process_results(results, attrs, base_func, args.graph, args.no_diff)
 
 
 if __name__ == '__main__':
@@ -152,6 +153,8 @@ if __name__ == '__main__':
                         help='IFUNC variant to set as baseline.')
     parser.add_argument('-g', '--graph', action='store_true',
                         help='Generate a graph from results.')
+    parser.add_argument('--no-diff', action='store_true',
+                        help='Do not include diff on .')
 
     args = parser.parse_args()
     main(args)
-- 
2.17.0

Re: [PATCH 1/3] benchtests: Add --no-diff option to avoid diff calculation

[Siddhesh Poyarekar]

On 05/25/2018 09:28 PM, leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Having the report with no diff numbers (percentages numbers
> enclosed within parenthesis) makes column separation cleaner
> (just spaces), allowing easier interaction with other tools.
> 
> 	* benchtests/scripts/compare_string.py: Add --no-diff
> 	option to avoid diff calculation.
> ---
>   ChangeLog                             |  5 +++++
>   benchtests/scripts/compare_strings.py | 15 +++++++++------
>   2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index c7436c4f333..afddd77315a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
> +
> +	* benchtests/scripts/compare_string.py: Add --no-diff option
> +	to avoid diff calculation.
> +
>   2018-05-24  Tulio Magno Quites Machado Filho  <tuliom@linux.ibm.com>
>   	    Gabriel F. T. Gomes  <gabriel@inconstante.eti.br>
>   
> diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
> index d37442076bf..ceeccea13b6 100755
> --- a/benchtests/scripts/compare_strings.py
> +++ b/benchtests/scripts/compare_strings.py
> @@ -79,7 +79,7 @@ def draw_graph(f, v, ifuncs, results):
>       pylab.savefig('%s-%s.png' % (f, v), bbox_inches='tight')
>   
>   
> -def process_results(results, attrs, base_func, graph):
> +def process_results(results, attrs, base_func, graph, no_diff):
>       """ Process results and print them
>   
>       Args:
> @@ -107,10 +107,11 @@ def process_results(results, attrs, base_func, graph):
>               graph_res[key] = res['timings']
>               for t in res['timings']:
>                   sys.stdout.write ('%12.2f' % t)
> -                if i != base_index:
> -                    base = res['timings'][base_index]
> -                    diff = (base - t) * 100 / base
> -                    sys.stdout.write (' (%6.2f%%)' % diff)
> +                if not no_diff:
> +                    if i != base_index:
> +                        base = res['timings'][base_index]
> +                        diff = (base - t) * 100 / base
> +                        sys.stdout.write (' (%6.2f%%)' % diff)
>                   sys.stdout.write('\t')
>                   i = i + 1
>               print('')
> @@ -132,7 +133,7 @@ def main(args):
>       attrs = args.attributes.split(',')
>   
>       results = parse_file(args.input, args.schema)
> -    process_results(results, attrs, base_func, args.graph)
> +    process_results(results, attrs, base_func, args.graph, args.no_diff)
>   
>   
>   if __name__ == '__main__':
> @@ -152,6 +153,8 @@ if __name__ == '__main__':
>                           help='IFUNC variant to set as baseline.')
>       parser.add_argument('-g', '--graph', action='store_true',
>                           help='Generate a graph from results.')
> +    parser.add_argument('--no-diff', action='store_true',
> +                        help='Do not include diff on .')

Please change the help description to "Do not print the difference from 
baseline" to be clearer.  OK with that change.

Thanks,
Siddhesh

>   
>       args = parser.parse_args()
>       main(args)
> 

[PATCH 2/3] benchtests: Add --no-header option to omit header

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Having raw output without header makes easier the interaction with
other tools, thus include an option to omit it.

	* benchtests/scripts/compare_string.py: Add --no-header
	option to omit header
---
 ChangeLog                             |  5 +++++
 benchtests/scripts/compare_strings.py | 17 +++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index afddd77315a..cc3e50e3b8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
+
+	* benchtests/scripts/compare_string.py: Add --no-header	option
+	to omit header.
+
 2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
 
 	* benchtests/scripts/compare_string.py: Add --no-diff option
diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
index ceeccea13b6..c629bba77fa 100755
--- a/benchtests/scripts/compare_strings.py
+++ b/benchtests/scripts/compare_strings.py
@@ -79,7 +79,7 @@ def draw_graph(f, v, ifuncs, results):
     pylab.savefig('%s-%s.png' % (f, v), bbox_inches='tight')
 
 
-def process_results(results, attrs, base_func, graph, no_diff):
+def process_results(results, attrs, base_func, graph, no_diff, no_header):
     """ Process results and print them
 
     Args:
@@ -88,16 +88,19 @@ def process_results(results, attrs, base_func, graph, no_diff):
     """
 
     for f in results['functions'].keys():
-        print('Function: %s' % f)
+
         v = results['functions'][f]['bench-variant']
-        print('Variant: %s' % v)
 
         base_index = 0
         if base_func:
             base_index = results['functions'][f]['ifuncs'].index(base_func)
 
-        print("%36s%s" % (' ', '\t'.join(results['functions'][f]['ifuncs'])))
-        print("=" * 120)
+        if not no_header:
+            print('Function: %s' % f)
+            print('Variant: %s' % v)
+            print("%36s%s" % (' ', '\t'.join(results['functions'][f]['ifuncs'])))
+            print("=" * 120)
+
         graph_res = {}
         for res in results['functions'][f]['results']:
             attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
@@ -133,7 +136,7 @@ def main(args):
     attrs = args.attributes.split(',')
 
     results = parse_file(args.input, args.schema)
-    process_results(results, attrs, base_func, args.graph, args.no_diff)
+    process_results(results, attrs, base_func, args.graph, args.no_diff, args.no_header)
 
 
 if __name__ == '__main__':
@@ -155,6 +158,8 @@ if __name__ == '__main__':
                         help='Generate a graph from results.')
     parser.add_argument('--no-diff', action='store_true',
                         help='Do not include diff on .')
+    parser.add_argument('--no-header', action='store_true',
+                        help='Do not include the header.')
 
     args = parser.parse_args()
     main(args)
-- 
2.17.0

Re: [PATCH 2/3] benchtests: Add --no-header option to omit header

[Siddhesh Poyarekar]

On 05/25/2018 09:28 PM, leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Having raw output without header makes easier the interaction with
> other tools, thus include an option to omit it.
> 
> 	* benchtests/scripts/compare_string.py: Add --no-header
> 	option to omit header
> ---
>   ChangeLog                             |  5 +++++
>   benchtests/scripts/compare_strings.py | 17 +++++++++++------
>   2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index afddd77315a..cc3e50e3b8d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
> +
> +	* benchtests/scripts/compare_string.py: Add --no-header	option
> +	to omit header.
> +
>   2018-05-25  Leonardo Sandoval  <leonardo.sandoval.gonzalez@intel.com>
>   
>   	* benchtests/scripts/compare_string.py: Add --no-diff option
> diff --git a/benchtests/scripts/compare_strings.py b/benchtests/scripts/compare_strings.py
> index ceeccea13b6..c629bba77fa 100755
> --- a/benchtests/scripts/compare_strings.py
> +++ b/benchtests/scripts/compare_strings.py
> @@ -79,7 +79,7 @@ def draw_graph(f, v, ifuncs, results):
>       pylab.savefig('%s-%s.png' % (f, v), bbox_inches='tight')
>   
>   
> -def process_results(results, attrs, base_func, graph, no_diff):
> +def process_results(results, attrs, base_func, graph, no_diff, no_header):
>       """ Process results and print them
>   
>       Args:
> @@ -88,16 +88,19 @@ def process_results(results, attrs, base_func, graph, no_diff):
>       """
>   
>       for f in results['functions'].keys():
> -        print('Function: %s' % f)
> +
>           v = results['functions'][f]['bench-variant']
> -        print('Variant: %s' % v)
>   
>           base_index = 0
>           if base_func:
>               base_index = results['functions'][f]['ifuncs'].index(base_func)
>   
> -        print("%36s%s" % (' ', '\t'.join(results['functions'][f]['ifuncs'])))
> -        print("=" * 120)
> +        if not no_header:
> +            print('Function: %s' % f)
> +            print('Variant: %s' % v)
> +            print("%36s%s" % (' ', '\t'.join(results['functions'][f]['ifuncs'])))
> +            print("=" * 120)
> +
>           graph_res = {}
>           for res in results['functions'][f]['results']:
>               attr_list = ['%s=%s' % (a, res[a]) for a in attrs]
> @@ -133,7 +136,7 @@ def main(args):
>       attrs = args.attributes.split(',')
>   
>       results = parse_file(args.input, args.schema)
> -    process_results(results, attrs, base_func, args.graph, args.no_diff)
> +    process_results(results, attrs, base_func, args.graph, args.no_diff, args.no_header)
>   
>   
>   if __name__ == '__main__':
> @@ -155,6 +158,8 @@ if __name__ == '__main__':
>                           help='Generate a graph from results.')
>       parser.add_argument('--no-diff', action='store_true',
>                           help='Do not include diff on .')
> +    parser.add_argument('--no-header', action='store_true',
> +                        help='Do not include the header.')

Please change the help text to "Do not include the header in the output".

Also, if you're writing a tool to analyze the results, please also 
consider propose merging them into glibc if you think they could be of 
more general use.

Thanks,
Siddhesh

>   
>       args = parser.parse_args()
>       main(args)
>