[PATCH] Simplify crud() in ident.c

[PATCH] Simplify crud() in ident.c

[Alex Riesen]

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

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

Re: [PATCH] Simplify crud() in ident.c

[Jakub Narebski]

Perhaps simplier, but isn't it slower?

-- 
Jakub Narebski

Re: [PATCH] Simplify crud() in ident.c

[Alex Riesen]

Jakub Narebski, Mon, Dec 03, 2007 21:19:29 +0100:
> Perhaps simplier, but isn't it slower?

doubt it

Re: [PATCH] Simplify crud() in ident.c

[Luke Lu]

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

Re: [PATCH] Simplify crud() in ident.c

[Johannes Schindelin]

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

Re: [PATCH] Simplify crud() in ident.c

[Alex Riesen]

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.

Re: [PATCH] Simplify crud() in ident.c

[Johannes Schindelin]

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

Re: [PATCH] Simplify crud() in ident.c

[David Kastrup]

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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