threads / patch / 11123

patchSimplify crud() in ident.c

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

## tl;dr

8 messages between Dec 3, 2007 and Dec 3, 2007. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Alex Riesen· Dec 3, 2007, 19:11 UTC · lore
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Noticed it accidentally.
 ident.c |   28 +++++++++-------------------
 1 files changed, 9 insertions(+), 19 deletions(-)
Show changes to ident.c +9 −19
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
Luke Lu· Dec 3, 2007, 22:55 UTC · re: Jakub Narebski · lore

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

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
Johannes Schindelin· Dec 3, 2007, 20:47 UTC · re: Alex Riesen · lore

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

Hi,
On Mon, 3 Dec 2007, Alex Riesen wrote:
Show 36 quoted lines
> 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· Dec 3, 2007, 21:37 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin, Mon, Dec 03, 2007 21:47:09 +0100:
Show 13 quoted lines
> 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· Dec 3, 2007, 22:48 UTC · re: Alex Riesen · lore

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

Hi,
On Mon, 3 Dec 2007, Alex Riesen wrote:
Show 20 quoted lines
> 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· Dec 3, 2007, 22:52 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 26 quoted lines
> 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

← back to recent threads