lib: small update for strlen, strnlen, use less cpu instructions

PATCH In review Orestes Leal Rodriguez 2 participants 3 messages Jun 16, 2015 – Jun 19, 2015

Subsystems (2)

Library code Generic string library

lore

Share & tools

lore

Orestes Leal Rodriguez

author

2015-06-16 21:59 UTC #0 lore raw

PATCH 1/1 lib: small update for strlen, strnlen, use less cpu instructions

Quoted 1 line

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

Quoted 1 line

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.

Signed-off-by Orestes Leal Rodriguez <[email protected]>

---

Signed-off-by Orestes Leal Rodriguez <[email protected]>

lib/string.c +13 -8

index 992bf30..c873436 100644
@@ -17,6 +17,10 @@
  * * Sat Feb 09 2002, Jason Thomas <[email protected]>,
  *                    Matthew Hawkins <[email protected]>
  * -  Kissed strtok() goodbye
+ *
+ * * Tuesday June 16 2015, Orestes Leal Rodriguez <[email protected]>
+ * - strlen, strnlen: by using a single counter we use less cpu instructions
+ *   by avoiding substracting the memory addresses before return
  */
 
 #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 */;
+	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 */;
+	return sz;
 }
+
 EXPORT_SYMBOL(strnlen);
 #endif

Richard Weinberger

2015-06-17 20:07 UTC #1 lore raw

Am 17.06.2015 um 00:51 schrieb Orestes Leal Rodriguez:

Quoted 4 lines

Quoted 1 line
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.

Quoted 19 lines

Quoted 1 line
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:

Quoted 5 lines

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

Quoted 5 lines

Signed-off-by Orestes Leal Rodriguez <[email protected]>
---
Signed-off-by Orestes Leal Rodriguez <[email protected]>

What does this 2nd SoB here?

lib/string.c line 23

Quoted 12 lines

lib/string.c +4 -0

index 992bf30..c873436 100644
@@ -17,6 +17,10 @@
  * * Sat Feb 09 2002, Jason Thomas <[email protected]>,
  *                    Matthew Hawkins <[email protected]>
  * -  Kissed strtok() goodbye
+ *
+ * * Tuesday June 16 2015, Orestes Leal Rodriguez <[email protected]>
+ * - 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. :-)

Quoted 5 lines

lib/string.c +0 -0

  */
 
 #include <linux/types.h>

@@ -401,11 +405,11 @@ EXPORT_SYMBOL(strim);
  */

lib/string.c EXPORT_SYMBOL(strim); · line 411

Quoted 10 lines

lib/string.c +3 -4

 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>

Quoted 6 lines

lib/string.c +1 -0

+    return sz;
 }
 EXPORT_SYMBOL(strlen);
 #endif

@@ -418,12 +422,13 @@ EXPORT_SYMBOL(strlen);
  */

lib/string.c EXPORT_SYMBOL(strlen); · line 428

Quoted 10 lines

lib/string.c +3 -4

 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

Orestes Leal Rodriguez

author

2015-06-19 03:11 UTC #2 lore raw

thank you for your comments, begginer mistakes (I guess),

Quoted 9 lines · Richard Weinberger

Am 17.06.2015 um 00:51 schrieb Orestes Leal Rodriguez:
Quoted 5 lines
Quoted 1 line
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.

Quoted 25 lines

Quoted 25 lines
Quoted 2 lines
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

Quoted 6 lines

Quoted 1 line
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

Quoted 8 lines

Quoted 6 lines
Signed-off-by Orestes Leal Rodriguez <[email protected]>
---
Signed-off-by Orestes Leal Rodriguez <[email protected]>
What does this 2nd SoB here?

Again, confusion about where to put the sob.

Quoted 16 lines

Quoted 12 lines
lib/string.c +3 -0
index 992bf30..c873436 100644
@@ -17,6 +17,10 @@
  * * Sat Feb 09 2002, Jason Thomas <[email protected]>,
  *                    Matthew Hawkins <[email protected]>
  * -  Kissed strtok() goodbye
+ *
+ * * Tuesday June 16 2015, Orestes Leal Rodriguez <[email protected]>
+ * - strlen, strnlen: by using a single counter we use less cpu
Quoted 2 lines
instructions
+ * by avoiding substracting the memory addresses before return
No need to add anything here. These days we have git. :-)

Ups!

Quoted 11 lines

#include <linux/types.h>

lib/string.c +1 -1

@@ -401,11 +405,11 @@ EXPORT_SYMBOL(strim);
  */
 size_t strlen(const char *s)
 {
-    const char *sc;
+    size_t sz = 0;

Quoted 9 lines

lib/string.c EXPORT_SYMBOL(strim); · line 411
Quoted 5 lines
lib/string.c +2 -3
-    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.

Quoted 20 lines

Quoted 7 lines

lib/string.c +1 -0

+    return sz;
 }
 EXPORT_SYMBOL(strlen);
 #endif

@@ -418,12 +422,13 @@ EXPORT_SYMBOL(strlen);
  */

lib/string.c EXPORT_SYMBOL(strlen); · line 428

Quoted 10 lines

lib/string.c +3 -4

 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.

Quoted 3 lines

Thanks,
//R2D2

Thanks to you richard.