]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
This patch cleans up the strsep implementation and improves performance.
authorWilco Dijkstra <wdijkstr@arm.com>
Wed, 21 Dec 2016 15:16:29 +0000 (15:16 +0000)
committerWilco Dijkstra <wdijkstr@arm.com>
Wed, 21 Dec 2016 15:16:29 +0000 (15:16 +0000)
Currently strsep calls strpbrk is is now a veneer to strcspn.  Calling
strcspn directly is faster.  Since it handles a delimiter string of size
1 as a special case, this is not needed in strsep itself.  Although this
means there is a slightly higher overhead if the delimiter size is 1,
all other cases are slightly faster.  The overall performance gain is 5-10%
on AArch64.

The string/bits/string2.h header contains optimizations for constant
delimiters of size 1-3.  Benchmarking these showed similar performance for
size 1 (since in all cases strchr/strchrnul is used), while size 2 and 3
can give up to 2x speedup for small input strings.  However if these cases
are common it seems much better to add this optimization to strcspn.
So move these header optimizations to string-inlines.c.

Improve the strsep benchmark so that it actually benchmarks something.
The current version contains a delimiter character at every position in the
input string, so there is very little work to do, and the extremely inefficent
simple_strsep implementation appears fastest in every case.  The new version
has either no match in the input for the fail case and a match halfway in the
input for the success case.  The input is then restored so that each iteration
does exactly the same amount of work.  Reduce the number of testcases since
simple_strsep takes a lot of time now.

* benchtests/bench-strsep.c (oldstrsep): Add old implementation.
(do_one_test) Restore original string so iteration works.
* string/string-inlines.c (do_test): Create better input strings.
(test_main) Reduce number of testruns.
* string/string-inlines.c (__old_strsep_1c): New function.
(__old_strsep_2c): Likewise.
(__old_strsep_3c): Likewise.
* string/strsep.c (__strsep): Remove case of small delim string.
Call strcspn directly rather than strpbrk.
* string/bits/string2.h (__strsep): Remove define.
(__strsep_1c): Remove.
(__strsep_2c): Remove.
(__strsep_3c): Remove.
(strsep): Remove.
* sysdeps/unix/sysv/linux/internal_statvfs.c
(__statvfs_getflags): Rename to __strsep.

ChangeLog
benchtests/bench-strsep.c
string/bits/string2.h
string/string-inlines.c
string/strsep.c
sysdeps/unix/sysv/linux/internal_statvfs.c

index b8436d37f6770a7244b8eeeb7d16d9b60d7b3ecf..8e8c0a3f6b6041cc09a598f158e05f4164f298d8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2015-12-21  Wilco Dijkstra  <wdijkstr@arm.com>
+
+       * benchtests/bench-strsep.c (oldstrsep): Add old implementation.
+       (do_one_test) Restore original string so iteration works.
+       * string/string-inlines.c (do_test): Create better input strings.
+       (test_main) Reduce number of testruns.
+       * string/string-inlines.c (__old_strsep_1c): New function.
+       (__old_strsep_2c): Likewise.
+       (__old_strsep_3c): Likewise.
+       * string/strsep.c (__strsep): Remove case of small delim string.
+       Call strcspn directly rather than strpbrk.
+       * string/bits/string2.h (__strsep): Remove define.
+       (__strsep_1c): Remove.
+       (__strsep_2c): Remove.
+       (__strsep_3c): Remove.
+       (strsep): Remove.
+       * sysdeps/unix/sysv/linux/internal_statvfs.c
+       (__statvfs_getflags): Rename to __strsep.
+
 2016-12-21  Florian Weimer  <fweimer@redhat.com>
 
        * csu/libc-tls.c (_dl_tls_setup): Remove.
index 774a093431275670f6fd8ae6a8ceb7e3bb50a453..4a53b4f449b537b33308b485849cd73d91b9dc08 100644 (file)
@@ -49,10 +49,56 @@ simple_strsep (char **s1, char *s2)
   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)
@@ -63,7 +109,10 @@ do_one_test (impl_t * impl, const char *s1, const char *s2)
   TIMING_NOW (start);
   for (i = 0; i < iters; ++i)
     {
-      CALL (impl, &s1, s2);
+      const char *s1a = s1;
+      CALL (impl, &s1a, s2);
+      if (s1a != NULL)
+       ((char*)s1a)[-1] = '1';
     }
   TIMING_NOW (stop);
 
@@ -76,7 +125,10 @@ static void
 do_test (size_t align1, size_t align2, size_t len1, size_t len2, int fail)
 {
   char *s2 = (char *) (buf2 + align2);
-  static const char d[] = "1234567890abcdef";
+
+  /* Search for a delimiter in a string containing mostly '0', so don't
+     use '0' as a delimiter.  */
+  static const char d[] = "123456789abcdefg";
 #define dl (sizeof (d) - 1)
   char *ss2 = s2;
   for (size_t l = len2; l > 0; l = l > dl ? l - dl : 0)
@@ -92,24 +144,9 @@ do_test (size_t align1, size_t align2, size_t len1, size_t len2, int fail)
   FOR_EACH_IMPL (impl, 0)
   {
     char *s1 = (char *) (buf1 + align1);
-    if (fail)
-      {
-       char *ss1 = s1;
-       for (size_t l = len1; l > 0; l = l > dl ? l - dl : 0)
-         {
-           size_t t = l > dl ? dl : l;
-           memcpy (ss1, d, t);
-           ++ss1[len2 > 7 ? 7 : len2 - 1];
-           ss1 += t;
-         }
-      }
-    else
-      {
-       memset (s1, '0', len1);
-       memcpy (s1 + (len1 - len2) - 2, s2, len2);
-       if ((len1 / len2) > 4)
-         memcpy (s1 + (len1 - len2) - (3 * len2), s2, len2);
-      }
+    memset (s1, '0', len1);
+    if (!fail)
+      s1[len1 / 2] = '1';
     s1[len1] = '\0';
     do_one_test (impl, s1, s2);
   }
@@ -127,7 +164,7 @@ test_main (void)
   putchar ('\n');
 
   for (size_t klen = 2; klen < 32; ++klen)
-    for (size_t hlen = 2 * klen; hlen < 16 * klen; hlen += klen)
+    for (size_t hlen = 4 * klen; hlen < 8 * klen; hlen += klen)
       {
        do_test (0, 0, hlen, klen, 0);
        do_test (0, 0, hlen, klen, 1);
index e39d4f1a85c25a4f47418e6a5613b27177ca6cbb..c1366172faf80fc65e43f3f8e5646042b7df8be1 100644 (file)
@@ -180,96 +180,6 @@ extern void *__rawmemchr (const void *__s, int __c);
 #endif
 
 
-#if !defined _HAVE_STRING_ARCH_strsep || defined _FORCE_INLINES
-# ifndef _HAVE_STRING_ARCH_strsep
-
-extern char *__strsep_g (char **__stringp, const char *__delim);
-#  define __strsep(s, reject) \
-  __extension__                                                                      \
-  ({ char __r0, __r1, __r2;                                                  \
-     (__builtin_constant_p (reject) && __string2_1bptr_p (reject)            \
-      && (__r0 = ((const char *) (reject))[0],                               \
-         ((const char *) (reject))[0] != '\0')                               \
-      ? ((__r1 = ((const char *) (reject))[1],                               \
-        ((const char *) (reject))[1] == '\0')                                \
-        ? __strsep_1c (s, __r0)                                              \
-        : ((__r2 = ((const char *) (reject))[2], __r2 == '\0')               \
-           ? __strsep_2c (s, __r0, __r1)                                     \
-           : (((const char *) (reject))[3] == '\0'                           \
-              ? __strsep_3c (s, __r0, __r1, __r2)                            \
-              : __strsep_g (s, reject))))                                    \
-      : __strsep_g (s, reject)); })
-# endif
-
-__STRING_INLINE char *__strsep_1c (char **__s, char __reject);
-__STRING_INLINE char *
-__strsep_1c (char **__s, char __reject)
-{
-  char *__retval = *__s;
-  if (__retval != NULL && (*__s = strchr (__retval, __reject)) != NULL)
-    *(*__s)++ = '\0';
-  return __retval;
-}
-
-__STRING_INLINE char *__strsep_2c (char **__s, char __reject1, char __reject2);
-__STRING_INLINE char *
-__strsep_2c (char **__s, char __reject1, char __reject2)
-{
-  char *__retval = *__s;
-  if (__retval != NULL)
-    {
-      char *__cp = __retval;
-      while (1)
-       {
-         if (*__cp == '\0')
-           {
-             __cp = NULL;
-         break;
-           }
-         if (*__cp == __reject1 || *__cp == __reject2)
-           {
-             *__cp++ = '\0';
-             break;
-           }
-         ++__cp;
-       }
-      *__s = __cp;
-    }
-  return __retval;
-}
-
-__STRING_INLINE char *__strsep_3c (char **__s, char __reject1, char __reject2,
-                                  char __reject3);
-__STRING_INLINE char *
-__strsep_3c (char **__s, char __reject1, char __reject2, char __reject3)
-{
-  char *__retval = *__s;
-  if (__retval != NULL)
-    {
-      char *__cp = __retval;
-      while (1)
-       {
-         if (*__cp == '\0')
-           {
-             __cp = NULL;
-         break;
-           }
-         if (*__cp == __reject1 || *__cp == __reject2 || *__cp == __reject3)
-           {
-             *__cp++ = '\0';
-             break;
-           }
-         ++__cp;
-       }
-      *__s = __cp;
-    }
-  return __retval;
-}
-# ifdef __USE_MISC
-#  define strsep(s, reject) __strsep (s, reject)
-# endif
-#endif
-
 /* We need the memory allocation functions for inline strdup().
    Referring to stdlib.h (even minimally) is not allowed
    in any of the tight standards compliant modes.  */
index d43e5897c37430e5f97940469d65e7ddbdcbd09c..7b1de7180f8be3dca1515e23553a9ecbdc76f5e2 100644 (file)
@@ -63,6 +63,70 @@ __old_strtok_r_1c (char *__s, char __sep, char **__nextp)
   return __result;
 }
 compat_symbol (libc, __old_strtok_r_1c, __strtok_r_1c, GLIBC_2_1_1);
+
+char *
+__old_strsep_1c (char **__s, char __reject)
+{
+  char *__retval = *__s;
+  if (__retval != NULL && (*__s = strchr (__retval, __reject)) != NULL)
+    *(*__s)++ = '\0';
+  return __retval;
+}
+compat_symbol (libc, __old_strsep_1c, __strsep_1c, GLIBC_2_1_1);
+
+char *
+__old_strsep_2c (char **__s, char __reject1, char __reject2)
+{
+  char *__retval = *__s;
+  if (__retval != NULL)
+    {
+      char *__cp = __retval;
+      while (1)
+       {
+         if (*__cp == '\0')
+           {
+             __cp = NULL;
+             break;
+           }
+         if (*__cp == __reject1 || *__cp == __reject2)
+           {
+             *__cp++ = '\0';
+             break;
+           }
+         ++__cp;
+       }
+      *__s = __cp;
+    }
+  return __retval;
+}
+compat_symbol (libc, __old_strsep_2c, __strsep_2c, GLIBC_2_1_1);
+
+char *
+__old_strsep_3c (char **__s, char __reject1, char __reject2, char __reject3)
+{
+  char *__retval = *__s;
+  if (__retval != NULL)
+    {
+      char *__cp = __retval;
+      while (1)
+       {
+         if (*__cp == '\0')
+           {
+             __cp = NULL;
+             break;
+           }
+         if (*__cp == __reject1 || *__cp == __reject2 || *__cp == __reject3)
+           {
+             *__cp++ = '\0';
+             break;
+           }
+         ++__cp;
+       }
+      *__s = __cp;
+    }
+  return __retval;
+}
+compat_symbol (libc, __old_strsep_3c, __strsep_3c, GLIBC_2_1_1);
 #endif
 
 #if SHLIB_COMPAT (libc, GLIBC_2_1_1, GLIBC_2_24)
index 10547740481bed8f7a065f76eccfe7b320c3c49c..68581c863980c1e86ae8559400ca0d1cec80d95e 100644 (file)
@@ -29,30 +29,10 @@ __strsep (char **stringp, const char *delim)
   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);
+  /* Find the end of the token.  */
+  end = begin + strcspn (begin, delim);
 
-  if (end)
+  if (*end)
     {
       /* Terminate the token and set *STRINGP past NUL character.  */
       *end++ = '\0';
index 012f93fe66bde9958d4d5d3432859a33e778f69b..ce649cd28d9edd7ed4eef55ee336da21115a944a 100644 (file)
@@ -168,7 +168,7 @@ __statvfs_getflags (const char *name, int fstype, int fd)
              char *cp = mntbuf.mnt_opts;
              char *opt;
 
-             while ((opt = strsep (&cp, ",")) != NULL)
+             while ((opt = __strsep (&cp, ",")) != NULL)
                if (strcmp (opt, "ro") == 0)
                  result |= ST_RDONLY;
                else if (strcmp (opt, "nosuid") == 0)