Re: [PING] Wordexp benchtest: good, bad and unreliable.

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/16/2015 01:32 PM, OndÅej BÃlka wrote:
>> As I mentioned before for wordexp performance depends on too many
>> parameters. So here is microbenchmark that shows some improvements and
>> some regressions. There is lot of noise. I ran these three times and on
>> one * and ffffffffoo pattern become twice slower for no reason.

There is a reason, but we just don't know it yet.

>> So how do you decide with conflicted data like this?

You can't make an "automatic" or "automated" decision, but as a baseline
the statistics do help. With noisy benchmarks you can still look at
potential trends in the noise. We don't want the bad patterns to get 5x
slower.

Similarly we will have conflicting workloads modeled as benchmarks, and
the hard part is that as an expert we have to make a difficult choice.
We have to understand the workload and decide "Yes, we ignore this performance
decrease because we expect fewer people will suffer." However, in order
to do that we need comments about exactly what workload we're trying to model.

For most of our microbenchmarks the workloads are "raw performance of function
call" and that's very simplistic, it's also very generic.

>> old implementation
>>                        	wordexp
>> Pattern foo flags 0:	1081.77
>> Pattern ffoo flags 0:	621.516
>> Pattern ffffoo flags 0:	697.562
>> Pattern ffffffffoo flags 0:	867.922
>> Pattern ffffffffffffffffoo flags 0:	1110.75
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1567.92
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2541.03
>> Pattern * flags 0:	141070
>> Pattern foo flags 0:	1061.08
>> Pattern ffoo flags 0:	1190.48
>> Pattern ffffoo flags 0:	1146.83
>> Pattern ffffffffoo flags 0:	1407.77
>> Pattern ffffffffffffffffoo flags 0:	1734.33
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2143.02
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3100.41
>> Pattern * flags 0:	205510
>>
>>
>>                        	wordexp
>> Pattern foo flags 0:	1081.84
>> Pattern ffoo flags 0:	672.422
>> Pattern ffffoo flags 0:	704.828
>> Pattern ffffffffoo flags 0:	875.188
>> Pattern ffffffffffffffffoo flags 0:	1089.7
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1594.03
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2575.28
>> Pattern * flags 0:	144160
>> Pattern foo flags 0:	1062.45
>> Pattern ffoo flags 0:	1099.72
>> Pattern ffffoo flags 0:	1153.95
>> Pattern ffffffffoo flags 0:	1391.45
>> Pattern ffffffffffffffffoo flags 0:	1820.36
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2129
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3171.5
>> Pattern * flags 0:	208475
>>
>>
>>                        	wordexp
>> Pattern foo flags 0:	1086.2
>> Pattern ffoo flags 0:	615.078
>> Pattern ffffoo flags 0:	688.594
>> Pattern ffffffffoo flags 0:	851.984
>> Pattern ffffffffffffffffoo flags 0:	1085.48
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1562.95
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2540.08
>> Pattern * flags 0:	287636
>> Pattern foo flags 0:	1068.11
>> Pattern ffoo flags 0:	1092.5
>> Pattern ffffoo flags 0:	1143.7
>> Pattern ffffffffoo flags 0:	1380.66
>> Pattern ffffffffffffffffoo flags 0:	1763.88
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2165.72
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3095.41
>> Pattern * flags 0:	207373
>>
>> new
>>
>>
>>                        	wordexp
>> Pattern foo flags 0:	1161.95
>> Pattern ffoo flags 0:	631.016
>> Pattern ffffoo flags 0:	714.797
>> Pattern ffffffffoo flags 0:	2209.84
>> Pattern ffffffffffffffffoo flags 0:	1024.17
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1408.22
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2223.39
>> Pattern * flags 0:	146925
>> Pattern foo flags 0:	1660.66
>> Pattern ffoo flags 0:	1673.34
>> Pattern ffffoo flags 0:	1730.23
>> Pattern ffffffffoo flags 0:	2030.42
>> Pattern ffffffffffffffffoo flags 0:	2245.59
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2512.95
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3312.95
>> Pattern * flags 0:	210492
>>
>>
>>                        	wordexp
>> Pattern foo flags 0:	1184.19
>> Pattern ffoo flags 0:	634.062
>> Pattern ffffoo flags 0:	683.688
>> Pattern ffffffffoo flags 0:	826.891
>> Pattern ffffffffffffffffoo flags 0:	1020.27
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1405.89
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2185.44
>> Pattern * flags 0:	140257
>> Pattern foo flags 0:	1649.77
>> Pattern ffoo flags 0:	1668.39
>> Pattern ffffoo flags 0:	1722.23
>> Pattern ffffffffoo flags 0:	1924.27
>> Pattern ffffffffffffffffoo flags 0:	2322.56
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2502.8
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3306.39
>> Pattern * flags 0:	208003
>>
>>
>>                        	wordexp
>> Pattern foo flags 0:	1097.7
>> Pattern ffoo flags 0:	655.891
>> Pattern ffffoo flags 0:	692.453
>> Pattern ffffffffoo flags 0:	860.203
>> Pattern ffffffffffffffffoo flags 0:	1009.38
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	1426.02
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	2179.64
>> Pattern * flags 0:	144492
>> Pattern foo flags 0:	1668.47
>> Pattern ffoo flags 0:	1684.38
>> Pattern ffffoo flags 0:	1718.75
>> Pattern ffffffffoo flags 0:	1932.91
>> Pattern ffffffffffffffffoo flags 0:	2294.95
>> Pattern ffffffffffffffffffffffffffffffffoo flags 0:	2514.89
>> Pattern ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo flags 0:	3329.14
>> Pattern * flags 0:	211152
>>
>> diff --git a/benchtests/Makefile b/benchtests/Makefile
>> index 8e615e5..fb737da 100644
>> --- a/benchtests/Makefile
>> +++ b/benchtests/Makefile
>> @@ -35,7 +35,7 @@ string-bench := bcopy bzero memccpy memchr memcmp memcpy memmem memmove \
>>  		strcat strchr strchrnul strcmp strcpy strcspn strlen \
>>  		strncasecmp strncat strncmp strncpy strnlen strpbrk strrchr \
>>  		strspn strstr strcpy_chk stpcpy_chk memrchr strsep strtok \
>> -		strcoll
>> +		strcoll wordexp
>>  string-bench-all := $(string-bench)
>>  
>>  # We have to generate locales
>> diff --git a/benchtests/bench-wordexp.c b/benchtests/bench-wordexp.c
>> new file mode 100644
>> index 0000000..ffadbcc
>> --- /dev/null
>> +++ b/benchtests/bench-wordexp.c
>> @@ -0,0 +1,97 @@
>> +/* Measure wordexp functions.
>> +   Copyright (C) 2015 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#define TEST_MAIN
>> +#define TEST_NAME "wordexp"
>> +#include "bench-string.h"
>> +
>> +#include <wordexp.h>
>> +
>> +typedef int (*proto_t) (const char *, wordexp_t *, int);
>> +
>> +IMPL (wordexp, 1)
>> +
>> +
>> +static void
>> +do_one_test (impl_t *impl, const char *s1, int flag)
>> +{
>> +  size_t i, iters = INNER_LOOP_ITERS;
>> +  timing_t start, stop, cur;
>> +
>> +  TIMING_NOW (start);
>> +  for (i = 0; i < iters; ++i)
>> +    {
>> +      wordexp_t out;
>> +      CALL (impl, s1, &out, flag);
>> +    }
>> +  TIMING_NOW (stop);
>> +
>> +  TIMING_DIFF (cur, start, stop);
>> +
>> +  TIMING_PRINT_MEAN ((double) cur, (double) iters);
>> +}
>> +
>> +
>> +static void
>> +do_test (const char *s1, int flag)
>> +{
>> +
>> +  printf ("Pattern %s flags %i:", s1, flag);
>> +
>> +  FOR_EACH_IMPL (impl, 0)
>> +    do_one_test (impl, s1, flag);
>> +
>> +  putchar ('\n');
>> +}
>> +
>> +static int
>> +test_main (void)
>> +{
>> +  test_init ();
>> +
>> +  printf ("%23s", "");
>> +  FOR_EACH_IMPL (impl, 0)
>> +    printf ("\t%s", impl->name);
>> +  putchar ('\n');
>> +
>> +  do_test ("foo", 0);
>> +  do_test ("ffoo", 0);
>> +  do_test ("ffffoo", 0);
>> +  do_test ("ffffffffoo", 0);
>> +  do_test ("ffffffffffffffffoo", 0);
>> +  do_test ("ffffffffffffffffffffffffffffffffoo", 0);
>> +do_test ("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo", 0);

This group of tests should have a comment about what's it's testing.

What workload are we modeling?

>> +
>> +
>> +  do_test ("*", 0);
>> +  setenv("IFS","                                                  \
>> +                                               ", 1);
>> +
>> +  do_test ("foo", 0);
>> +  do_test ("ffoo", 0);
>> +  do_test ("ffffoo", 0);
>> +  do_test ("ffffffffoo", 0);
>> +  do_test ("ffffffffffffffffoo", 0);
>> +  do_test ("ffffffffffffffffffffffffffffffffoo", 0);
>> +do_test ("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffoo", 0);
>> +  do_test ("*", 0);
>> +

Similarly with the IFS change. This is not a normal IFS change for an average workload.

Usually one changes IFS to be a series of characters, or just tab, or just newline etc.

>> +  return ret;
>> +}
>> +
>> +#include "../test-skeleton.c"
> 

The benchmark looks OK to me modulo the added comments.

Cheers,
Carlos.