# [PATCH] Simplify crud() in ident.c 8 messages from 2007-12-03 to 2007-12-03. Participants: Alex Riesen, Jakub Narebski, Johannes Schindelin, David Kastrup, Luke Lu. Thread: https://gitlist.dev/t/11123 ## Alex Riesen, 2007-12-03 19:11 Subject: [PATCH] Simplify crud() in ident.c Message-ID: <20071203191143.GB17671@steel.home> URL: https://gitlist.dev/e/20071203191143.GB17671%40steel.home ``` Signed-off-by: Alex Riesen --- Noticed it accidentally. ident.c | 28 +++++++++------------------- 1 files changed, 9 insertions(+), 19 deletions(-) diff --git a/ident.c b/ident.c index 9b2a852..dbd0f52 100644 --- a/ident.c +++ b/ident.c @@ -113,25 +113,15 @@ static int add_raw(char *buf, size_t size, int offset, const char *str) static int crud(unsigned char c) { - static char crud_array[256]; - static int crud_array_initialized = 0; - - if (!crud_array_initialized) { - int k; - - for (k = 0; k <= 31; ++k) crud_array[k] = 1; - crud_array[' '] = 1; - crud_array['.'] = 1; - crud_array[','] = 1; - crud_array[':'] = 1; - crud_array[';'] = 1; - crud_array['<'] = 1; - crud_array['>'] = 1; - crud_array['"'] = 1; - crud_array['\''] = 1; - crud_array_initialized = 1; - } - return crud_array[c]; + return c <= 32 || + c == '.' || + c == ',' || + c == ':' || + c == ';' || + c == '<' || + c == '>' || + c == '"' || + c == '\''; } /* -- 1.5.3.6.1022.g35305 ``` ## Jakub Narebski, 2007-12-03 20:19 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: URL: https://gitlist.dev/e/m3lk8bv8du.fsf%40roke.D-201 In-Reply-To: <20071203191143.GB17671@steel.home> ``` Perhaps simplier, but isn't it slower? -- Jakub Narebski ``` ## Johannes Schindelin, 2007-12-03 20:47 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0712032046480.27959%40racer.site In-Reply-To: <20071203191143.GB17671@steel.home> ``` Hi, On Mon, 3 Dec 2007, Alex Riesen wrote: > diff --git a/ident.c b/ident.c > index 9b2a852..dbd0f52 100644 > --- a/ident.c > +++ b/ident.c > @@ -113,25 +113,15 @@ static int add_raw(char *buf, size_t size, int offset, const char *str) > > static int crud(unsigned char c) > { > - static char crud_array[256]; > - static int crud_array_initialized = 0; > - > - if (!crud_array_initialized) { > - int k; > - > - for (k = 0; k <= 31; ++k) crud_array[k] = 1; > - crud_array[' '] = 1; > - crud_array['.'] = 1; > - crud_array[','] = 1; > - crud_array[':'] = 1; > - crud_array[';'] = 1; > - crud_array['<'] = 1; > - crud_array['>'] = 1; > - crud_array['"'] = 1; > - crud_array['\''] = 1; > - crud_array_initialized = 1; > - } > - return crud_array[c]; > + return c <= 32 || > + c == '.' || > + c == ',' || > + c == ':' || > + c == ';' || > + c == '<' || > + c == '>' || > + c == '"' || > + c == '\''; Or enhance ctype.c. Ciao, Dscho ``` ## Alex Riesen, 2007-12-03 21:32 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: <20071203213247.GC17671@steel.home> URL: https://gitlist.dev/e/20071203213247.GC17671%40steel.home In-Reply-To: ``` Jakub Narebski, Mon, Dec 03, 2007 21:19:29 +0100: > Perhaps simplier, but isn't it slower? doubt it ``` ## Alex Riesen, 2007-12-03 21:37 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: <20071203213702.GD17671@steel.home> URL: https://gitlist.dev/e/20071203213702.GD17671%40steel.home In-Reply-To: ``` Johannes Schindelin, Mon, Dec 03, 2007 21:47:09 +0100: > On Mon, 3 Dec 2007, Alex Riesen wrote: > > + return c <= 32 || > > + c == '.' || > > + c == ',' || > > + c == ':' || > > + c == ';' || > > + c == '<' || > > + c == '>' || > > + c == '"' || > > + c == '\''; > > Or enhance ctype.c. > That's be nice, but the "crud" conflicts with existing classification, so I'd have to change the is*-macros as well. Don't feel like it. I believe the code is never in hotpath anyway so the shorter the better. ``` ## Johannes Schindelin, 2007-12-03 22:48 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0712032247140.27959%40racer.site In-Reply-To: <20071203213702.GD17671@steel.home> ``` Hi, On Mon, 3 Dec 2007, Alex Riesen wrote: > Johannes Schindelin, Mon, Dec 03, 2007 21:47:09 +0100: > > On Mon, 3 Dec 2007, Alex Riesen wrote: > > > + return c <= 32 || > > > + c == '.' || > > > + c == ',' || > > > + c == ':' || > > > + c == ';' || > > > + c == '<' || > > > + c == '>' || > > > + c == '"' || > > > + c == '\''; > > > > Or enhance ctype.c. > > > > That's be nice, but the "crud" conflicts with existing classification, > so I'd have to change the is*-macros as well. Don't feel like it. > > I believe the code is never in hotpath anyway so the shorter the > better. Really? return !!strchr(".,:;<>\"\\", c); Ciao, Dscho ``` ## David Kastrup, 2007-12-03 22:52 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: <85zlwre6i2.fsf@lola.goethe.zz> URL: https://gitlist.dev/e/85zlwre6i2.fsf%40lola.goethe.zz In-Reply-To: ``` Johannes Schindelin writes: > On Mon, 3 Dec 2007, Alex Riesen wrote: > >> Johannes Schindelin, Mon, Dec 03, 2007 21:47:09 +0100: >> > On Mon, 3 Dec 2007, Alex Riesen wrote: >> > > + return c <= 32 || >> > > + c == '.' || >> > > + c == ',' || >> > > + c == ':' || >> > > + c == ';' || >> > > + c == '<' || >> > > + c == '>' || >> > > + c == '"' || >> > > + c == '\''; >> > >> > Or enhance ctype.c. >> > >> >> That's be nice, but the "crud" conflicts with existing classification, >> so I'd have to change the is*-macros as well. Don't feel like it. >> >> I believe the code is never in hotpath anyway so the shorter the >> better. > > Really? > > return !!strchr(".,:;<>\"\\", c); What happened to c <= 32? -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ``` ## Luke Lu, 2007-12-03 22:55 Subject: Re: [PATCH] Simplify crud() in ident.c Message-ID: <903B0E5A-32E2-4029-90C1-8C227BEDD03D@vicaya.com> URL: https://gitlist.dev/e/903B0E5A-32E2-4029-90C1-8C227BEDD03D%40vicaya.com In-Reply-To: ``` On Dec 3, 2007, at 12:19 PM, Jakub Narebski wrote: > Perhaps simplier, but isn't it slower? Actually it's faster on modern cpu with deep pipelines. The following is simple test on my macbookpro (repeated 3 times and picked lowest one): $ time ./crudtest 1000000000 old crud... real 0m0.856s user 0m0.839s sys 0m0.011s $ time ./crudtest 1000000000 simple new crud... real 0m0.431s user 0m0.421s sys 0m0.007s Note: it's compiled with gcc -O2. -O3 gives the same timing; -O: simple crud has the same timing while the old crud is 10x slower; -O0 (off): the simple code is 50% slower than old crud (note: 10x less iterations): $ time ./crudtest0 100000000 old crud... real 0m0.659s user 0m0.638s sys 0m0.008s $ time ./crudtest0 100000000 simple new crud... real 0m1.175s user 0m1.149s sys 0m0.014s Since the default CFLAGS in git Makefile has -O2, the simple/new code is faster by default. __Luke ```