diff mbox

Improve memccpy performance

Message ID 001f01d03004$e05abdb0$a1103910$@com
State New
Headers show

Commit Message

Wilco Jan. 14, 2015, 2:17 p.m. UTC
Improve memccpy performance by using memchr/memcpy rather than a byte loop. 
Overall performance on bench-memccpy is > 2x faster when using the C 
implementation of memchr and an optimized memcpy.

ChangeLog:

2015-01-14  Wilco Dijkstra  wdijkstr@arm.com

    * string/memccpy.c (memccpy): Improve performance using memchr/memcpy.

---
 string/memccpy.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Roland McGrath Jan. 14, 2015, 6:14 p.m. UTC | #1
> +      return memcpy (dest, src, n) + n;

Use __mempcpy here.
Richard Earnshaw Jan. 15, 2015, 10:45 a.m. UTC | #2
On 14/01/15 18:14, Roland McGrath wrote:
>> +      return memcpy (dest, src, n) + n;
> 
> Use __mempcpy here.
> 
That will be worse if mempcpy just calls memcpy; which is what the C
library implementation does.

R.
Wilco Jan. 15, 2015, 2:28 p.m. UTC | #3
> Richard Earnshaw wrote: 
> On 14/01/15 18:14, Roland McGrath wrote:
> >> +      return memcpy (dest, src, n) + n;
> >
> > Use __mempcpy here.
> >
> That will be worse if mempcpy just calls memcpy; which is what the C
> library implementation does.

If GLIBC inlines mempcpy like I proposed then it would be reasonable
to use mempcpy here as it results in exactly the same code.

Wilco
Wilco April 15, 2015, 12:31 p.m. UTC | #4
ping

> Wilco Dijkstra wrote: 
> > Wilco Dijkstra wrote:
> > > Richard Earnshaw wrote:
> > > On 14/01/15 18:14, Roland McGrath wrote:
> > > >> +      return memcpy (dest, src, n) + n;
> > > >
> > > > Use __mempcpy here.
> > > >
> > > That will be worse if mempcpy just calls memcpy; which is what the C
> > > library implementation does.
> >
> > If GLIBC inlines mempcpy like I proposed then it would be reasonable
> > to use mempcpy here as it results in exactly the same code.
> 
> So, OK for trunk with __mempcpy like below?
> 
> ---
>  string/memccpy.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/string/memccpy.c b/string/memccpy.c
> index 70ee2ae..d4146f9 100644
> --- a/string/memccpy.c
> +++ b/string/memccpy.c
> @@ -26,15 +26,15 @@
>  void *
>  __memccpy (void *dest, const void *src, int c, size_t n)
>  {
> -  const char *s = src;
> -  char *d = dest;
> -  const char x = c;
> -  size_t i = n;
> +  void *p = memchr (src, c, n);
> 
> -  while (i-- > 0)
> -    if ((*d++ = *s++) == x)
> -      return d;
> +  if (p != NULL)
> +    {
> +      n = p - src + 1;
> +      return __mempcpy (dest, src, n);
> +    }
> 
> +  memcpy (dest, src, n);
>    return NULL;
>  }
> 
> --
> 1.9.1
Ondřej Bílka May 12, 2015, 9:21 a.m. UTC | #5
On Wed, Apr 15, 2015 at 01:31:30PM +0100, Wilco Dijkstra wrote:
> ping
> 
> > Wilco Dijkstra wrote: 
> > > Wilco Dijkstra wrote:
> > > > Richard Earnshaw wrote:
> > > > On 14/01/15 18:14, Roland McGrath wrote:
> > > > >> +      return memcpy (dest, src, n) + n;
> > > > >
> > > > > Use __mempcpy here.
> > > > >
> > > > That will be worse if mempcpy just calls memcpy; which is what the C
> > > > library implementation does.
> > >
> > > If GLIBC inlines mempcpy like I proposed then it would be reasonable
> > > to use mempcpy here as it results in exactly the same code.
> > 
> > So, OK for trunk with __mempcpy like below?
> > 
> > ---
> >  string/memccpy.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/string/memccpy.c b/string/memccpy.c
> > index 70ee2ae..d4146f9 100644
> > --- a/string/memccpy.c
> > +++ b/string/memccpy.c
> > @@ -26,15 +26,15 @@
> >  void *
> >  __memccpy (void *dest, const void *src, int c, size_t n)
> >  {
> > -  const char *s = src;
> > -  char *d = dest;
> > -  const char x = c;
> > -  size_t i = n;
> > +  void *p = memchr (src, c, n);
> > 
> > -  while (i-- > 0)
> > -    if ((*d++ = *s++) == x)
> > -      return d;
> > +  if (p != NULL)
> > +    {
> > +      n = p - src + 1;
> > +      return __mempcpy (dest, src, n);
> > +    }
> > 
> > +  memcpy (dest, src, n);
> >    return NULL;
> >  }
> > 
> > --
> > 1.9.1

ok for me.
Wilco July 6, 2015, 11:30 a.m. UTC | #6
> Wilco Dijkstra wrote:
> ping
> 
> > Wilco Dijkstra wrote:
> > > Wilco Dijkstra wrote:
> > > > Richard Earnshaw wrote:
> > > > On 14/01/15 18:14, Roland McGrath wrote:
> > > > >> +      return memcpy (dest, src, n) + n;
> > > > >
> > > > > Use __mempcpy here.
> > > > >
> > > > That will be worse if mempcpy just calls memcpy; which is what the C
> > > > library implementation does.
> > >
> > > If GLIBC inlines mempcpy like I proposed then it would be reasonable
> > > to use mempcpy here as it results in exactly the same code.
> >
> > So, OK for trunk with __mempcpy like below?
> >
> > ---
> >  string/memccpy.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/string/memccpy.c b/string/memccpy.c
> > index 70ee2ae..d4146f9 100644
> > --- a/string/memccpy.c
> > +++ b/string/memccpy.c
> > @@ -26,15 +26,15 @@
> >  void *
> >  __memccpy (void *dest, const void *src, int c, size_t n)
> >  {
> > -  const char *s = src;
> > -  char *d = dest;
> > -  const char x = c;
> > -  size_t i = n;
> > +  void *p = memchr (src, c, n);
> >
> > -  while (i-- > 0)
> > -    if ((*d++ = *s++) == x)
> > -      return d;
> > +  if (p != NULL)
> > +    {
> > +      n = p - src + 1;
> > +      return __mempcpy (dest, src, n);
> > +    }
> >
> > +  memcpy (dest, src, n);
> >    return NULL;
> >  }
> >
> > --
> > 1.9.1
diff mbox

Patch

diff --git a/string/memccpy.c b/string/memccpy.c
index 70ee2ae..d4146f9 100644
--- a/string/memccpy.c
+++ b/string/memccpy.c
@@ -26,15 +26,15 @@ 
 void *
 __memccpy (void *dest, const void *src, int c, size_t n)
 {
-  const char *s = src;
-  char *d = dest;
-  const char x = c;
-  size_t i = n;
+  void *p = memchr (src, c, n);
 
-  while (i-- > 0)
-    if ((*d++ = *s++) == x)
-      return d;
+  if (p != NULL)
+    {
+      n = p - src + 1;
+      return memcpy (dest, src, n) + n;
+    }
 
+  memcpy (dest, src, n);
   return NULL;
 }