Re: [RFC PATCH] benchtests:Add BENCHSET list of targets

["Rodriguez Bahena, Victor" <victor dot rodriguez dot bahena at intel dot com>]

-----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