Patchwork Improve bench-strlen

login
register
mail settings
Submitter Wilco Dijkstra
Date Dec. 21, 2018, 2:56 p.m.
Message ID <DB5PR08MB103004B8B2973917D6791AA183B80@DB5PR08MB1030.eurprd08.prod.outlook.com>
Download mbox | patch
Permalink /patch/30790/
State New
Headers show

Comments

Wilco Dijkstra - Dec. 21, 2018, 2:56 p.m.
The current bench-strlen compares against a slow byte-oriented strlen which
is not useful given it's too easy to beat.  Remove it and compare against the
generic C strlen version and memchr.

OK to commit?

ChangeLog:
2018-12-21  Wilco Dijkstra  <wdijkstr@arm.com>

        * benchtests/bench-strlen.c (generic_strlen): New function.
        (memchr_strlen): New function.

--
Siddhesh Poyarekar - Dec. 21, 2018, 3:30 p.m.
On 21/12/18 8:26 PM, Wilco Dijkstra wrote:
> The current bench-strlen compares against a slow byte-oriented strlen which
> is not useful given it's too easy to beat.  Remove it and compare against the
> generic C strlen version and memchr.
> 
> OK to commit?

OK.  We should do this for all functions.

Siddhesh
Wilco Dijkstra - Dec. 21, 2018, 3:44 p.m.
Hi Siddhesh,
  
>On 21/12/18 8:26 PM, Wilco Dijkstra wrote:
>> The current bench-strlen compares against a slow byte-oriented strlen which
>> is not useful given it's too easy to beat.  Remove it and compare against the
>> generic C strlen version and memchr.
>> 
>> OK to commit?
>
> OK.  We should do this for all functions.

Absolutely. I already did a few more, but wanted to get one out for review to
get some feedback. I'm also planning to remove the "stupid_xxx" variants as
they don't make sense at all.

The other thing we need to do is increase iterations to get more accurate results
and sort out the odd buffer handling from page counts to bytes (with the default
increased so you don't have to request more in many cases). And then we can
look into improving the actual tests run, for example I already mentioned we run
too many combinations in various tests (particularly unaligned) for the results to
be meaningful.

Cheers,
Wilco
Siddhesh Poyarekar - Dec. 21, 2018, 3:54 p.m.
On 21/12/18 9:14 PM, Wilco Dijkstra wrote:
> Absolutely. I already did a few more, but wanted to get one out for review to
> get some feedback. I'm also planning to remove the "stupid_xxx" variants as
> they don't make sense at all.

Agreed.

> The other thing we need to do is increase iterations to get more accurate results
> and sort out the odd buffer handling from page counts to bytes (with the default
> increased so you don't have to request more in many cases). And then we can
> look into improving the actual tests run, for example I already mentioned we run
> too many combinations in various tests (particularly unaligned) for the results to
> be meaningful.

Sounds good.

Siddhesh

Patch

diff --git a/benchtests/bench-strlen.c b/benchtests/bench-strlen.c
index 4c26890e23c2e08e2237cf0327cc4b0e4b2f2259..00429cc6b72826443f84f64f30f1774f16394bd8 100644
--- a/benchtests/bench-strlen.c
+++ b/benchtests/bench-strlen.c
@@ -24,24 +24,15 @@ 
 #endif
 #include "bench-string.h"
 
-#ifndef WIDE
-# define MAX_CHAR CHAR_MAX
-#else
-# define MAX_CHAR WCHAR_MAX
-#endif
-
 #include "json-lib.h"
 
 typedef size_t (*proto_t) (const CHAR *);
 
-size_t
-simple_STRLEN (const CHAR *s)
-{
-  const CHAR *p;
+size_t generic_strlen (const CHAR *);
+size_t memchr_strlen (const CHAR *);
 
-  for (p = s; *p; ++p);
-  return p - s;
-}
+IMPL (memchr_strlen, 0)
+IMPL (generic_strlen, 0)
 
 #ifndef WIDE
 size_t
@@ -52,7 +43,12 @@  builtin_strlen (const CHAR *p)
 IMPL (builtin_strlen, 0)
 #endif
 
-IMPL (simple_STRLEN, 0)
+size_t
+memchr_strlen (const CHAR *p)
+{
+  return (const CHAR *)MEMCHR (p, 0, SIZE_MAX/2) - p;
+}
+
 IMPL (STRLEN, 1)
 
 
@@ -161,3 +157,13 @@  test_main (void)
 }
 
 #include <support/test-driver.c>
+
+#define libc_hidden_builtin_def(X)
+#ifndef WIDE
+# undef STRLEN
+# define STRLEN generic_strlen
+# include <string/strlen.c>
+#else
+# define WCSLEN generic_strlen
+# include <wcsmbs/wcslen.c>
+#endif