Re: [PATCH] Benchtests: Remove simple_strcspn/strpbrk/strsep

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 08/03/23 08:05, Wilco Dijkstra via Libc-alpha wrote:
> 
> Remove simple_strcspn/strpbrk/strsep which are significantly slower than the generic
> implementations. Also remove oldstrsep and oldstrtok since they are practically identical
> to the generic implementation.  Adjust iteration count to reduce benchmark time.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


> 
> ---
> 
> diff --git a/benchtests/bench-strcspn.c b/benchtests/bench-strcspn.c
> index 2388f33d5441b302ffe34a9bb4ee7ed70db7306b..fa74d305ae66c8f220b2f35d8733520a62a84968 100644
> --- a/benchtests/bench-strcspn.c
> +++ b/benchtests/bench-strcspn.c
> @@ -26,29 +26,8 @@
>  #endif /* WIDE */
>  #include "bench-string.h"
>  
> -#ifndef WIDE
> -# define SIMPLE_STRCSPN simple_strcspn
> -#else
> -# define SIMPLE_STRCSPN simple_wcscspn
> -#endif /* WIDE */
> -
>  typedef size_t (*proto_t) (const CHAR *, const CHAR *);
> -size_t SIMPLE_STRCSPN (const CHAR *, const CHAR *);
>  
> -IMPL (SIMPLE_STRCSPN, 0)
>  IMPL (STRCSPN, 1)
>  
> -size_t
> -SIMPLE_STRCSPN (const CHAR *s, const CHAR *rej)
> -{
> -  const CHAR *r, *str = s;
> -  CHAR c;
> -
> -  while ((c = *s++) != '\0')
> -    for (r = rej; *r != '\0'; ++r)
> -      if (*r == c)
> -	return s - str - 1;
> -  return s - str - 1;
> -}
> -
>  #include "bench-strpbrk.c"
> diff --git a/benchtests/bench-strpbrk.c b/benchtests/bench-strpbrk.c
> index e96fb0efbf06fcb6c329917f73a90149d2bcb944..55199b73c9233bcedf52bd8e45dca628371c2803 100644
> --- a/benchtests/bench-strpbrk.c
> +++ b/benchtests/bench-strpbrk.c
> @@ -35,31 +35,10 @@
>  # endif /* WIDE */
>  # include "bench-string.h"
>  
> -# ifndef WIDE
> -#  define SIMPLE_STRPBRK simple_strpbrk
> -# else
> -#  define SIMPLE_STRPBRK simple_wcspbrk
> -# endif /* WIDE */
> -
>  typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
> -CHAR *SIMPLE_STRPBRK (const CHAR *, const CHAR *);
>  
> -IMPL (SIMPLE_STRPBRK, 0)
>  IMPL (STRPBRK, 1)
>  
> -CHAR *
> -SIMPLE_STRPBRK (const CHAR *s, const CHAR *rej)
> -{
> -  const CHAR *r;
> -  CHAR c;
> -
> -  while ((c = *s++) != '\0')
> -    for (r = rej; *r != '\0'; ++r)
> -      if (*r == c)
> -	return (CHAR *) s - 1;
> -  return NULL;
> -}
> -
>  #endif /* !STRPBRK_RESULT */
>  
>  #include "json-lib.h"
> @@ -69,7 +48,7 @@ do_one_test (json_ctx_t *json_ctx, impl_t *impl, const CHAR *s,
>               const CHAR *rej, RES_TYPE exp_res)
>  {
>    RES_TYPE res = CALL (impl, s, rej);
> -  size_t i, iters = INNER_LOOP_ITERS;
> +  size_t i, iters = INNER_LOOP_ITERS8 / CHARBYTES;
>    timing_t start, stop, cur;
>  
>    if (res != exp_res)
> diff --git a/benchtests/bench-strsep.c b/benchtests/bench-strsep.c
> index 4b203aff30db1592120db5f0b39b06a6e9cd4a13..d7df3fa0a9aae04918ad54fc7927a1da18ceb402 100644
> --- a/benchtests/bench-strsep.c
> +++ b/benchtests/bench-strsep.c
> @@ -20,90 +20,14 @@
>  #define TEST_NAME "strsep"
>  #include "bench-string.h"
>  
> -char *
> -simple_strsep (char **s1, char *s2)
> -{
> -  char *begin;
> -  char *s;
> -  size_t j = 0;
> -
> -  begin = *s1;
> -  s = begin;
> -  if (begin == NULL)
> -    return NULL;
> -  ssize_t s2len = strlen (s2);
> -  while (*s)
> -    {
> -      for (j = 0; j < s2len; j++)
> -	{
> -	  if (*s == s2[j])
> -	    {
> -	      s[0] = '\0';
> -	      *s1 = s + 1;
> -	      return begin;
> -	    }
> -	}
> -      s++;
> -    }
> -  *s1 = NULL;
> -  return begin;
> -}
> -
> -char *
> -oldstrsep (char **stringp, const char *delim)
> -{
> -  char *begin, *end;
> -
> -  begin = *stringp;
> -  if (begin == NULL)
> -    return NULL;
> -
> -  /* A frequent case is when the delimiter string contains only one
> -     character.  Here we don't need to call the expensive `strpbrk'
> -     function and instead work using `strchr'.  */
> -  if (delim[0] == '\0' || delim[1] == '\0')
> -    {
> -      char ch = delim[0];
> -
> -      if (ch == '\0')
> -	end = NULL;
> -      else
> -	{
> -	  if (*begin == ch)
> -	    end = begin;
> -	  else if (*begin == '\0')
> -	    end = NULL;
> -	  else
> -	    end = strchr (begin + 1, ch);
> -	}
> -    }
> -  else
> -    /* Find the end of the token.  */
> -    end = strpbrk (begin, delim);
> -
> -  if (end)
> -    {
> -      /* Terminate the token and set *STRINGP past NUL character.  */
> -      *end++ = '\0';
> -      *stringp = end;
> -    }
> -  else
> -    /* No more delimiters; this is the last token.  */
> -    *stringp = NULL;
> -
> -  return begin;
> -}
> -
>  typedef char *(*proto_t) (const char **, const char *);
>  
> -IMPL (simple_strsep, 0)
>  IMPL (strsep, 1)
> -IMPL (oldstrsep, 2)
>  
>  static void
>  do_one_test (impl_t * impl, const char *s1, const char *s2)
>  {
> -  size_t i, iters = INNER_LOOP_ITERS_SMALL;
> +  size_t i, iters = INNER_LOOP_ITERS;
>    timing_t start, stop, cur;
>  
>    TIMING_NOW (start);
> diff --git a/benchtests/bench-strspn.c b/benchtests/bench-strspn.c
> index 707613cbcf5ac556e6d5a0c3a95c19be1dcadfc6..cc98ffb38bd5d7085efa8c1d1fb7aba6c46c1b3f 100644
> --- a/benchtests/bench-strspn.c
> +++ b/benchtests/bench-strspn.c
> @@ -28,41 +28,20 @@
>  #define BIG_CHAR MAX_CHAR
>  
>  #ifndef WIDE
> -# define SIMPLE_STRSPN simple_strspn
>  # define SMALL_CHAR 127
>  #else
> -# define SIMPLE_STRSPN simple_wcsspn
>  # define SMALL_CHAR 1273
>  #endif /* WIDE */
>  
>  typedef size_t (*proto_t) (const CHAR *, const CHAR *);
> -size_t SIMPLE_STRSPN (const CHAR *, const CHAR *);
>  
> -IMPL (SIMPLE_STRSPN, 0)
>  IMPL (STRSPN, 1)
>  
> -size_t
> -SIMPLE_STRSPN (const CHAR *s, const CHAR *acc)
> -{
> -  const CHAR *r, *str = s;
> -  CHAR c;
> -
> -  while ((c = *s++) != '\0')
> -    {
> -      for (r = acc; *r != '\0'; ++r)
> -	if (*r == c)
> -	  break;
> -      if (*r == '\0')
> -	return s - str - 1;
> -    }
> -  return s - str - 1;
> -}
> -
>  static void
>  do_one_test (json_ctx_t *json_ctx, impl_t *impl, const CHAR *s,
>               const CHAR *acc, size_t exp_res)
>  {
> -  size_t res = CALL (impl, s, acc), i, iters = INNER_LOOP_ITERS;
> +  size_t res = CALL (impl, s, acc), i, iters = INNER_LOOP_ITERS8 / CHARBYTES;
>    timing_t start, stop, cur;
>  
>    if (res != exp_res)
> diff --git a/benchtests/bench-strtok.c b/benchtests/bench-strtok.c
> index 711bdaab58ff297855b03d62057cefd066f4e1c5..b5789d7bf243c40810f38efd40f0bb9c870327ca 100644
> --- a/benchtests/bench-strtok.c
> +++ b/benchtests/bench-strtok.c
> @@ -20,47 +20,14 @@
>  #define TEST_NAME "strtok"
>  #include "bench-string.h"
>  
> -char *
> -oldstrtok (char *s, const char *delim)
> -{
> -  static char *olds;
> -  char *token;
> -
> -  if (s == NULL)
> -    s = olds;
> -
> -  /* Scan leading delimiters.  */
> -  s += strspn (s, delim);
> -  if (*s == '\0')
> -    {
> -      olds = s;
> -      return NULL;
> -    }
> -
> -  /* Find the end of the token.  */
> -  token = s;
> -  s = strpbrk (token, delim);
> -  if (s == NULL)
> -    /* This token finishes the string.  */
> -    olds = strchr (token, '\0');
> -  else
> -    {
> -      /* Terminate the token and make OLDS point past it.  */
> -      *s = '\0';
> -      olds = s + 1;
> -    }
> -  return token;
> -}
> -
>  typedef char *(*proto_t) (const char *, const char *);
>  
> -IMPL (oldstrtok, 0)
>  IMPL (strtok, 1)
>  
>  static void
>  do_one_test (impl_t * impl, const char *s1, const char *s2)
>  {
> -  size_t i, iters = INNER_LOOP_ITERS_SMALL;
> +  size_t i, iters = INNER_LOOP_ITERS_MEDIUM;
>    timing_t start, stop, cur;
>    TIMING_NOW (start);
>    for (i = 0; i < iters; ++i)
> @@ -74,7 +41,6 @@ do_one_test (impl_t * impl, const char *s1, const char *s2)
>    TIMING_DIFF (cur, start, stop);
>  
>    TIMING_PRINT_MEAN ((double) cur, (double) iters);
> -
>  }
>  
>  
>