Messages in this thread | | | Date | Thu, 18 Jun 2015 22:11:28 -0500 | Subject | Re: [PATCH 1/1] lib: small update for strlen, strnlen, use less cpu instructions | From | Orestes Leal Rodriguez <> |
| |
thank you for your comments, begginer mistakes (I guess),
On 6/17/15, Richard Weinberger <richard@nod.at> wrote: > Am 17.06.2015 um 00:51 schrieb Orestes Leal Rodriguez: >>> Use the force^Wcheckpatch.pl. >> This is the output of checkpatch.pl: >> output of checkpatch: total: 0 errors, 0 warnings, 42 lines checked >> /root/string.c.patch has no obvious style problems and is ready for >> submission > > But it does not apply at all. > Did you test it? I fear your mail client did some whitespace damage.
yes, I patched my original source tree with it.
> >>> You need to explain that in the commit message, my young padawan. >> Very small update to strlen and strnlen that now use less cpu instructions >> by using a counter to avoid memory address >> arithmetic, which cause that the compiler adds more machine >> instructions for computing the length of the string just before >> returning from the functions, the old machine code is like the >> following: >> >> mov -0x4(%ebp),%edx >> mov 0x8(%ebp),%eax >> sub %eax,%edx >> mov %edx,%eax >> leave >> ret >> >> >> now in the new versions the value is not calculated anymore, >> instead he value of the counter is put on eax after the >> condition inside the loop no longer holds, and then return: >> >> mov -0x4(%ebp),%eax >> leave >> ret >> >> With this a few cpu instructions are saved. > > x86_32 does not matter here as we have already an optimized strlen() in > arch/x86/lib/string_32.c. > Did you check whether the optimization is worth on other archs? > Hint: grep __HAVE_ARCH_STRLEN
I only have access to x86 cpus, but thanks for the hint
> >> >> Signed-off-by: Orestes Leal Rodriguez <lukes357@gmail.com> >> --- >> >> Signed-off-by: Orestes Leal Rodriguez <lukes357@gmail.com> > > What does this 2nd SoB here?
Again, confusion about where to put the sob.
> >> diff --git a/lib/string.c b/lib/string.c >> index 992bf30..c873436 100644 >> --- a/lib/string.c >> +++ b/lib/string.c >> @@ -17,6 +17,10 @@ >> * * Sat Feb 09 2002, Jason Thomas <jason@topic.com.au>, >> * Matthew Hawkins <matt@mh.dropbear.id.au> >> * - Kissed strtok() goodbye >> + * >> + * * Tuesday June 16 2015, Orestes Leal Rodriguez <lukes357@gmail.com> >> + * - strlen, strnlen: by using a single counter we use less cpu >> instructions >> + * by avoiding substracting the memory addresses before return > > No need to add anything here. These days we have git. :-)
Ups!
> >> */ >> >> #include <linux/types.h> >> @@ -401,11 +405,11 @@ EXPORT_SYMBOL(strim); >> */ >> size_t strlen(const char *s) >> { >> - const char *sc; >> + size_t sz = 0; >> >> - for (sc = s; *sc != '\0'; ++sc) >> - /* nothing */; >> - return sc - s; >> + for (; *s++ != '\0'; sz++) >> + /* empty */; > > <nitpick> > Why suddenly "empty" instead of "nothing"? > </nitpick>
I was bored.
> >> + return sz; >> } >> EXPORT_SYMBOL(strlen); >> #endif >> @@ -418,12 +422,13 @@ EXPORT_SYMBOL(strlen); >> */ >> size_t strnlen(const char *s, size_t count) >> { >> - const char *sc; >> + size_t sz = 0; >> >> - for (sc = s; count-- && *sc != '\0'; ++sc) >> - /* nothing */; >> - return sc - s; >> + for (; count-- && *s++ != '\0'; sz++) >> + /* empty */; > > Same here. > > Thanks, > //R2D2 >
Thanks to you richard.
| |