[RFC] benchtests:Add BENCHSET list of targets

Message ID 20171109035404.1237-1-victor.rodriguez.bahena@intel.com
State New, archived
Headers

Commit Message

Victor Rodriguez Nov. 9, 2017, 3:54 a.m. UTC
  This patch adds BENCHSET list of targets in order to run benchmarks as:

make bench BENCHSET="bench-pthread bench-math malloc-thread"

This helps users to run benchmarks acording to the glibc area they
are measuring.

Changelog:
2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>

       (VERSION): Set to 2.26
       * benchtests/Makefile:Add BENCHSET to allow subsets of benchmarks to be run

Signed-off-by: Victor Rodriguez <victor.rodriguez.bahena@intel.com>
Signed-off-by: Icarus Sparry <icarus.w.sparry@intel.com>
---
 ChangeLog           |  5 +++++
 benchtests/Makefile | 12 ++++++++++++
 2 files changed, 17 insertions(+)
  

Comments

Siddhesh Poyarekar Nov. 14, 2017, 3:36 p.m. UTC | #1
Thanks, the code change itself is good.  There are a couple of minor
nits below to fix that you can include in your next iteration.
Additionally, this needs documentation, so please add a note on this
option to benchtests/README describing it similar to other make variable
options in that file.

On Thursday 09 November 2017 09:24 AM, Victor Rodriguez wrote:
> This patch adds BENCHSET list of targets in order to run benchmarks as:
> 
> make bench BENCHSET="bench-pthread bench-math malloc-thread"

I suppose you meant bench-malloc-thread there.

> 
> This helps users to run benchmarks acording to the glibc area they
> are measuring.
> 
> Changelog:
> 2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>
> 
>        (VERSION): Set to 2.26

Copy paste error? :)

>        * benchtests/Makefile:Add BENCHSET to allow subsets of benchmarks to be run

End statement with a period.

> 
> Signed-off-by: Victor Rodriguez <victor.rodriguez.bahena@intel.com>
> Signed-off-by: Icarus Sparry <icarus.w.sparry@intel.com>
> ---
>  ChangeLog           |  5 +++++
>  benchtests/Makefile | 12 ++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 8dbfc7e..832461c 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>
> +
> +	(VERSION): Set to 2.26
> +	* benchtests/Makefile:Add BENCHSET to allow subsets of benchmarks to be run
> +
>  2017-08-02  Siddhesh Poyarekar  <siddhesh@sourceware.org>
>  
>  	* version.h (RELEASE): Set to "stable"
> diff --git a/benchtests/Makefile b/benchtests/Makefile
> index 37788e8..2bc4a25 100644
> --- a/benchtests/Makefile
> +++ b/benchtests/Makefile
> @@ -29,7 +29,11 @@ bench-pthread := pthread_once thread_create
>  
>  bench-string := ffs ffsll
>  
> +ifeq (${BENCHSET},)
>  bench := $(bench-math) $(bench-pthread) $(bench-string)
> +else
> +bench := $(foreach B,$(filter bench-%,${BENCHSET}), ${${B}})
> +endif
>  
>  # String function benchmarks.
>  string-benchset := bcopy bzero memccpy memchr memcmp memcpy memmem memmove \
> @@ -66,8 +70,12 @@ stdio-common-benchset := sprintf
>  
>  math-benchset := math-inlines
>  
> +ifeq (${BENCHSET},)
>  benchset := $(string-benchset-all) $(stdlib-benchset) $(stdio-common-benchset) \
>  	    $(math-benchset)
> +else
> +benchset := $(foreach B,$(filter %-benchset,${BENCHSET}), ${${B}})
> +endif
>  
>  CFLAGS-bench-ffs.c += -fno-builtin
>  CFLAGS-bench-ffsll.c += -fno-builtin
> @@ -77,7 +85,11 @@ CFLAGS-bench-fminf.c += -fno-builtin
>  CFLAGS-bench-fmax.c += -fno-builtin
>  CFLAGS-bench-fmaxf.c += -fno-builtin
>  
> +ifeq (${BENCHSET},)
>  bench-malloc := malloc-thread
> +else
> +bench-malloc := $(foreach B,$(filter malloc-%,${BENCHSET}), ${${B}})
> +endif
>  
>  $(addprefix $(objpfx)bench-,$(bench-math)): $(libm)
>  $(addprefix $(objpfx)bench-,$(math-benchset)): $(libm)
> 

Siddhesh
  
Victor Rodriguez Nov. 18, 2017, 2:41 p.m. UTC | #2
-----Original Message-----
From: Siddhesh Poyarekar <siddhesh@gotplt.org>

Date: Tuesday, November 14, 2017 at 9:36 AM
To: Victor Rodriguez Bahena <victor.rodriguez.bahena@intel.com>,
"libc-alpha@sourceware.org" <libc-alpha@sourceware.org>
Cc: "Sparry, Icarus W" <icarus.w.sparry@intel.com>
Subject: Re: [RFC PATCH] benchtests:Add BENCHSET list of targets

>Thanks, the code change itself is good.  There are a couple of minor

>nits below to fix that you can include in your next iteration.

>Additionally, this needs documentation, so please add a note on this

>option to benchtests/README describing it similar to other make variable

>options in that file.

>

>On Thursday 09 November 2017 09:24 AM, Victor Rodriguez wrote:

>> This patch adds BENCHSET list of targets in order to run benchmarks as:

>> 

>> make bench BENCHSET="bench-pthread bench-math malloc-thread"

>

>I suppose you meant bench-malloc-thread there.

>

>> 

>> This helps users to run benchmarks acording to the glibc area they

>> are measuring.

>> 

>> Changelog:

>> 2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>

>> 

>>        (VERSION): Set to 2.26

>

>Copy paste error? :)

>

>>        * benchtests/Makefile:Add BENCHSET to allow subsets of

>>benchmarks to be run

>

>End statement with a period.



Thanks for the feedback Siddhesh. I sent a new series of patches to
improve this: 

[PATCH 3/3] benchtests:Enable BENCHSET to run subset of tests
	This patch with your recommendations, regarding to the
bench-malloc-thread we will need to change the internal names already
defended: 

https://github.com/bminor/glibc/blob/master/benchtests/Makefile#L84


I think that in order to keep it simple we could go with malloc-thread,
but I’m open to your feedback.

[PATCH 2/3] benchtests: Adjust valid and accepted properties
	Fix problem described in
https://sourceware.org/ml/libc-alpha/2017-10/msg01090.html about invalid
benchmark output: 'workload-spec2006.wrf’ that does not match any of the
regexes: '^[_a-zA-Z0-9]*$¹.

[PATCH 1/3] benchtests: Wide range of tests names in schema.json
	fix to adjust valid properties in the bench results ( also comping from
workload-spec2006.wrf

Let me know if you are ok with this solution, I have tested and works as
expected 

Regards

Victor

>

>> 

>> Signed-off-by: Victor Rodriguez <victor.rodriguez.bahena@intel.com>

>> Signed-off-by: Icarus Sparry <icarus.w.sparry@intel.com>

>> ---

>>  ChangeLog           |  5 +++++

>>  benchtests/Makefile | 12 ++++++++++++

>>  2 files changed, 17 insertions(+)

>> 

>> diff --git a/ChangeLog b/ChangeLog

>> index 8dbfc7e..832461c 100644

>> --- a/ChangeLog

>> +++ b/ChangeLog

>> @@ -1,3 +1,8 @@

>> +2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>

>> +

>> +	(VERSION): Set to 2.26

>> +	* benchtests/Makefile:Add BENCHSET to allow subsets of benchmarks to

>>be run

>> +

>>  2017-08-02  Siddhesh Poyarekar  <siddhesh@sourceware.org>

>>  

>>  	* version.h (RELEASE): Set to "stable"

>> diff --git a/benchtests/Makefile b/benchtests/Makefile

>> index 37788e8..2bc4a25 100644

>> --- a/benchtests/Makefile

>> +++ b/benchtests/Makefile

>> @@ -29,7 +29,11 @@ bench-pthread := pthread_once thread_create

>>  

>>  bench-string := ffs ffsll

>>  

>> +ifeq (${BENCHSET},)

>>  bench := $(bench-math) $(bench-pthread) $(bench-string)

>> +else

>> +bench := $(foreach B,$(filter bench-%,${BENCHSET}), ${${B}})

>> +endif

>>  

>>  # String function benchmarks.

>>  string-benchset := bcopy bzero memccpy memchr memcmp memcpy memmem

>>memmove \

>> @@ -66,8 +70,12 @@ stdio-common-benchset := sprintf

>>  

>>  math-benchset := math-inlines

>>  

>> +ifeq (${BENCHSET},)

>>  benchset := $(string-benchset-all) $(stdlib-benchset)

>>$(stdio-common-benchset) \

>>  	    $(math-benchset)

>> +else

>> +benchset := $(foreach B,$(filter %-benchset,${BENCHSET}), ${${B}})

>> +endif

>>  

>>  CFLAGS-bench-ffs.c += -fno-builtin

>>  CFLAGS-bench-ffsll.c += -fno-builtin

>> @@ -77,7 +85,11 @@ CFLAGS-bench-fminf.c += -fno-builtin

>>  CFLAGS-bench-fmax.c += -fno-builtin

>>  CFLAGS-bench-fmaxf.c += -fno-builtin

>>  

>> +ifeq (${BENCHSET},)

>>  bench-malloc := malloc-thread

>> +else

>> +bench-malloc := $(foreach B,$(filter malloc-%,${BENCHSET}), ${${B}})

>> +endif

>>  

>>  $(addprefix $(objpfx)bench-,$(bench-math)): $(libm)

>>  $(addprefix $(objpfx)bench-,$(math-benchset)): $(libm)

>> 

>

>Siddhesh
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 8dbfc7e..832461c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2017-11-08  Victor Rodriguez  <victor.rodriguez.bahena@intel.com>
+
+	(VERSION): Set to 2.26
+	* benchtests/Makefile:Add BENCHSET to allow subsets of benchmarks to be run
+
 2017-08-02  Siddhesh Poyarekar  <siddhesh@sourceware.org>
 
 	* version.h (RELEASE): Set to "stable"
diff --git a/benchtests/Makefile b/benchtests/Makefile
index 37788e8..2bc4a25 100644
--- a/benchtests/Makefile
+++ b/benchtests/Makefile
@@ -29,7 +29,11 @@  bench-pthread := pthread_once thread_create
 
 bench-string := ffs ffsll
 
+ifeq (${BENCHSET},)
 bench := $(bench-math) $(bench-pthread) $(bench-string)
+else
+bench := $(foreach B,$(filter bench-%,${BENCHSET}), ${${B}})
+endif
 
 # String function benchmarks.
 string-benchset := bcopy bzero memccpy memchr memcmp memcpy memmem memmove \
@@ -66,8 +70,12 @@  stdio-common-benchset := sprintf
 
 math-benchset := math-inlines
 
+ifeq (${BENCHSET},)
 benchset := $(string-benchset-all) $(stdlib-benchset) $(stdio-common-benchset) \
 	    $(math-benchset)
+else
+benchset := $(foreach B,$(filter %-benchset,${BENCHSET}), ${${B}})
+endif
 
 CFLAGS-bench-ffs.c += -fno-builtin
 CFLAGS-bench-ffsll.c += -fno-builtin
@@ -77,7 +85,11 @@  CFLAGS-bench-fminf.c += -fno-builtin
 CFLAGS-bench-fmax.c += -fno-builtin
 CFLAGS-bench-fmaxf.c += -fno-builtin
 
+ifeq (${BENCHSET},)
 bench-malloc := malloc-thread
+else
+bench-malloc := $(foreach B,$(filter malloc-%,${BENCHSET}), ${${B}})
+endif
 
 $(addprefix $(objpfx)bench-,$(bench-math)): $(libm)
 $(addprefix $(objpfx)bench-,$(math-benchset)): $(libm)