Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On Fri, 23.08.13 14:09, WANG Chao (chaow...@redhat.com) wrote: > > > - Why not use startswith() instead of first_word()? > > first_word is not equivalent: there must be a word boundary after the > > word: > > Yeah, you're right. I occasionally got carried away at midnight :( > > And I think a inline static version could be a little faster? Right? Not necessarilly. On today's CPUs often smaller code results in faster execution than avoiding function calls... If we shift this around further we probably should do it properly and play around with benchmarks... Lennart -- Lennart Poettering - Red Hat, Inc. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On 08/22/13 at 06:35pm, Zbigniew Jędrzejewski-Szmek wrote:
> On Thu, Aug 22, 2013 at 11:03:11PM +0800, WANG Chao wrote:
> > On 08/22/13 at 06:53am, Zbigniew Jędrzejewski-Szmek wrote:
> > > On Thu, Aug 22, 2013 at 03:52:29AM +0200, Kay Sievers wrote:
> > > > On Wed, Aug 21, 2013 at 10:16 PM, Kay Sievers wrote:
> > > > > On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao
> > > > > wrote:
> > > > >> Take this optimized code from Lennart[1]. Now startswith is a macro
> > > > >> using
> > > > >> strncmp and should be fast enough.
> > > > >>
> > > > >> [1]:
> > > > >> http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html
> > > > >> ---
> > > > >> src/shared/macro.h | 5 +
> > > > >> src/shared/util.c | 17 -
> > > > >> 2 files changed, 5 insertions(+), 17 deletions(-)
> > > > >
> > > > > It fails to compile, there are the prototypes in util.h still.
> > > > > Also quite a few places do not include macro.h ...
> > > > >
> > > > > I guess it's easier and cleaner anyway to use simple static inline
> > > > > functions instead of defines. Patch attached.
> > > >
> > > > Zbigniew, mind checking if that patch makes sense?
> > > >
> > > > You added the hasprefix a while ago, and you also proposed a version
> > > > to unify the both.
> > > > We really should get that topic sorted out now. :)
> > > Looks correct, pushed this along with Shawn's patch.
> > >
> >
> > Hi, guys, I have two more questions:
> >
> > - Do we need to update endswith() to be static inline and use strncmp
> > instead of memcmp. It looks better if we put these two functions at
> > the same place (shared/util.h)
> Sounds useful.
>
> > - Why not use startswith() instead of first_word()?
> first_word is not equivalent: there must be a word boundary after the
> word:
Yeah, you're right. I occasionally got carried away at midnight :(
And I think a inline static version could be a little faster? Right?
>
> src/test/test-util.c:static void test_first_word(void) {
> src/test/test-util.c:assert_se(first_word("Hello", ""));
> src/test/test-util.c:assert_se(first_word("Hello", "Hello"));
> src/test/test-util.c:assert_se(first_word("Hello world", "Hello"));
> src/test/test-util.c:assert_se(first_word("Hello\tworld", "Hello"));
> src/test/test-util.c:assert_se(first_word("Hello\nworld", "Hello"));
> src/test/test-util.c:assert_se(first_word("Hello\rworld", "Hello"));
> src/test/test-util.c:assert_se(first_word("Hello ", "Hello"));
> src/test/test-util.c:assert_se(!first_word("Hello", "Hellooo"));
> src/test/test-util.c:assert_se(!first_word("Hello", "x"));
> src/test/test-util.c:assert_se(!first_word("Hellooo", "Hello"));
>
> Zbyszek
>
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On Thu, Aug 22, 2013 at 11:03:11PM +0800, WANG Chao wrote:
> On 08/22/13 at 06:53am, Zbigniew Jędrzejewski-Szmek wrote:
> > On Thu, Aug 22, 2013 at 03:52:29AM +0200, Kay Sievers wrote:
> > > On Wed, Aug 21, 2013 at 10:16 PM, Kay Sievers wrote:
> > > > On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao wrote:
> > > >> Take this optimized code from Lennart[1]. Now startswith is a macro
> > > >> using
> > > >> strncmp and should be fast enough.
> > > >>
> > > >> [1]:
> > > >> http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html
> > > >> ---
> > > >> src/shared/macro.h | 5 +
> > > >> src/shared/util.c | 17 -
> > > >> 2 files changed, 5 insertions(+), 17 deletions(-)
> > > >
> > > > It fails to compile, there are the prototypes in util.h still.
> > > > Also quite a few places do not include macro.h ...
> > > >
> > > > I guess it's easier and cleaner anyway to use simple static inline
> > > > functions instead of defines. Patch attached.
> > >
> > > Zbigniew, mind checking if that patch makes sense?
> > >
> > > You added the hasprefix a while ago, and you also proposed a version
> > > to unify the both.
> > > We really should get that topic sorted out now. :)
> > Looks correct, pushed this along with Shawn's patch.
> >
>
> Hi, guys, I have two more questions:
>
> - Do we need to update endswith() to be static inline and use strncmp
> instead of memcmp. It looks better if we put these two functions at
> the same place (shared/util.h)
Sounds useful.
> - Why not use startswith() instead of first_word()?
first_word is not equivalent: there must be a word boundary after the
word:
src/test/test-util.c:static void test_first_word(void) {
src/test/test-util.c:assert_se(first_word("Hello", ""));
src/test/test-util.c:assert_se(first_word("Hello", "Hello"));
src/test/test-util.c:assert_se(first_word("Hello world", "Hello"));
src/test/test-util.c:assert_se(first_word("Hello\tworld", "Hello"));
src/test/test-util.c:assert_se(first_word("Hello\nworld", "Hello"));
src/test/test-util.c:assert_se(first_word("Hello\rworld", "Hello"));
src/test/test-util.c:assert_se(first_word("Hello ", "Hello"));
src/test/test-util.c:assert_se(!first_word("Hello", "Hellooo"));
src/test/test-util.c:assert_se(!first_word("Hello", "x"));
src/test/test-util.c:assert_se(!first_word("Hellooo", "Hello"));
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On 08/22/13 at 06:53am, Zbigniew Jędrzejewski-Szmek wrote: > On Thu, Aug 22, 2013 at 03:52:29AM +0200, Kay Sievers wrote: > > On Wed, Aug 21, 2013 at 10:16 PM, Kay Sievers wrote: > > > On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao wrote: > > >> Take this optimized code from Lennart[1]. Now startswith is a macro using > > >> strncmp and should be fast enough. > > >> > > >> [1]: > > >> http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html > > >> --- > > >> src/shared/macro.h | 5 + > > >> src/shared/util.c | 17 - > > >> 2 files changed, 5 insertions(+), 17 deletions(-) > > > > > > It fails to compile, there are the prototypes in util.h still. > > > Also quite a few places do not include macro.h ... > > > > > > I guess it's easier and cleaner anyway to use simple static inline > > > functions instead of defines. Patch attached. > > > > Zbigniew, mind checking if that patch makes sense? > > > > You added the hasprefix a while ago, and you also proposed a version > > to unify the both. > > We really should get that topic sorted out now. :) > Looks correct, pushed this along with Shawn's patch. > Hi, guys, I have two more questions: - Do we need to update endswith() to be static inline and use strncmp instead of memcmp. It looks better if we put these two functions at the same place (shared/util.h) - Why not use startswith() instead of first_word()? Thanks WANG Chao ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On Thu, Aug 22, 2013 at 03:52:29AM +0200, Kay Sievers wrote: > On Wed, Aug 21, 2013 at 10:16 PM, Kay Sievers wrote: > > On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao wrote: > >> Take this optimized code from Lennart[1]. Now startswith is a macro using > >> strncmp and should be fast enough. > >> > >> [1]: > >> http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html > >> --- > >> src/shared/macro.h | 5 + > >> src/shared/util.c | 17 - > >> 2 files changed, 5 insertions(+), 17 deletions(-) > > > > It fails to compile, there are the prototypes in util.h still. > > Also quite a few places do not include macro.h ... > > > > I guess it's easier and cleaner anyway to use simple static inline > > functions instead of defines. Patch attached. > > Zbigniew, mind checking if that patch makes sense? > > You added the hasprefix a while ago, and you also proposed a version > to unify the both. > We really should get that topic sorted out now. :) Looks correct, pushed this along with Shawn's patch. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On Wed, Aug 21, 2013 at 10:16 PM, Kay Sievers wrote: > On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao wrote: >> Take this optimized code from Lennart[1]. Now startswith is a macro using >> strncmp and should be fast enough. >> >> [1]: >> http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html >> --- >> src/shared/macro.h | 5 + >> src/shared/util.c | 17 - >> 2 files changed, 5 insertions(+), 17 deletions(-) > > It fails to compile, there are the prototypes in util.h still. > Also quite a few places do not include macro.h ... > > I guess it's easier and cleaner anyway to use simple static inline > functions instead of defines. Patch attached. Zbigniew, mind checking if that patch makes sense? You added the hasprefix a while ago, and you also proposed a version to unify the both. We really should get that topic sorted out now. :) Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] Optimize startswith() to macro
On Wed, Aug 21, 2013 at 8:15 PM, WANG Chao wrote: > Take this optimized code from Lennart[1]. Now startswith is a macro using > strncmp and should be fast enough. > > [1]: http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html > --- > src/shared/macro.h | 5 + > src/shared/util.c | 17 - > 2 files changed, 5 insertions(+), 17 deletions(-) It fails to compile, there are the prototypes in util.h still. Also quite a few places do not include macro.h ... I guess it's easier and cleaner anyway to use simple static inline functions instead of defines. Patch attached. Thanks, Kay 0001-replace-startswith-loop-with-optimizable-inline-fuct.patch Description: Binary data ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] Optimize startswith() to macro
Take this optimized code from Lennart[1]. Now startswith is a macro using
strncmp and should be fast enough.
[1]: http://lists.freedesktop.org/archives/systemd-devel/2013-July/011860.html
---
src/shared/macro.h | 5 +
src/shared/util.c | 17 -
2 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 0d3ff1c..6053a0c 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -188,6 +188,11 @@ static inline size_t ALIGN_TO(size_t l, size_t ali) {
#define hasprefix(s, prefix) (memcmp(s, prefix, strlen(prefix)) == 0)
+#define startswith(s, prefix) ({ \
+size_t _l = strlen(prefix); \
+strncmp(s, prefix, _l) == 0 ? s + _l : NULL; \
+})
+
#define IOVEC_SET_STRING(i, s) \
do {\
struct iovec *_i = &(i);\
diff --git a/src/shared/util.c b/src/shared/util.c
index ca9c2eb..fc89cec 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -128,23 +128,6 @@ char* endswith(const char *s, const char *postfix) {
return (char*) s + sl - pl;
}
-char* startswith(const char *s, const char *prefix) {
-const char *a, *b;
-
-assert(s);
-assert(prefix);
-
-a = s, b = prefix;
-for (;;) {
-if (*b == 0)
-return (char*) a;
-if (*a != *b)
-return NULL;
-
-a++, b++;
-}
-}
-
char* startswith_no_case(const char *s, const char *prefix) {
const char *a, *b;
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
