Correct dir.c to compile on Solaris 9
PATCH 1/1 Correct dir.c to compile on Solaris 9
The compiler on my Solaris 9 system doesn't understand the array initialization syntax used here in dir.c.
--- dir.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
dir.c +5 -6
@@ -423,18 +423,17 @@ static int cmp_name(const void *p1, const void *p2)*/ static int simple_length (const char *match) { -const char special[256 ] = { - [0 ] =1 , ['?' ] =1 , - ['\\' ] =1 , ['*' ] =1 , - ['[' ] =1 - };int len =-1 ;for (;;) {unsigned char c = *match++; len++; -if (special[c]) +switch (c) { +case 0 :case '?' : +case '\\' :case '*' : +case '[' :return len; + } } }
Signature · 1 line
1.5.1.1.83.g2bfe3
Hi,
Quoted 25 lines · Shawn O. Pearce
The compiler on my Solaris 9 system doesn't understand the array initialization syntax used here in
dir.c.Signed-off-by Shawn O. Pearce <[email protected]>--- dir.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)dir.c +0
-5@@ -423,18 +423,17 @@ static int cmp_name(const void *p1, const void *p2)*/ static int simple_length (const char *match) { -const char special[256 ] = { - [0 ] =1 , ['?' ] =1 , - ['\\' ] =1 , ['*' ] =1 , - ['[' ] =1 - };int len =-1 ;for (;;) {unsigned char c = *match++;
dir.c static int cmp_name(const void *p1, const void *p2) · line 438
Quoted 10 lines
dir.c +5
-1len++; -if (special[c]) +switch (c) { +case 0 :case '?' : +case '\\' :case '*' : +case '[' :return len; + } } }
You are replacing a table-based check with a switch based, which might be substantially slower (depends on how often cmp_name() is called).
Maybe there is another way to initialize the table (and make it static to begin with)?
Ciao,
Dscho
Quoted 10 lines · Johannes Schindelin
Quoted 9 lines · Shawn O. Pearce
static int
simple_length(const char *match)
{dir.c +0
-5-const char special[256 ] = { - [0 ] =1 , ['?' ] =1 , - ['\\' ] =1 , ['*' ] =1 , - ['[' ] =1 - };int len =-1 ;
Quoted 15 lines
dir.c static int cmp_name(const void *p1, const void *p2) · line 438
Quoted 12 lines
dir.c +5
-1for (;;) {unsigned char c = *match++; len++; -if (special[c]) +switch (c) { +case 0 :case '?' : +case '\\' :case '*' : +case '[' :return len; + } } }You are replacing a table-based check with a switch based, which might be substantially slower (depends on how often
cmp_name()is called).
Or faster. When the table gives a cache miss and has to be loaded from main memory, I am quite sure that 5 compares in a row are faster than the cache miss.
Actually, with the switch, the compiler is free to implement it with a table (and gcc usually does this, probably even using a substantially smaller table). The table-based check in contrast looks like some kind of micro-optimization which makes the code IMHO more difficult to read, and which only would be justified with meassured improvements.
Josef
söndag 15 april 2007 23:03 skrev Josef Weidendorfer:
Quoted 27 lines · Johannes Schindelin
Quoted 10 lines · Shawn O. Pearce
Quoted 9 lines
static int
simple_length(const char *match)
{(quoted diff) +0
-5- const char special[256] = { - [0] = 1, ['?'] = 1, - ['\\'] = 1, ['*'] = 1, - ['['] = 1 - }; int len = -1;Quoted 15 lines
Review
Quoted 12 lines
(quoted diff) +5
-1for (;;) { unsigned char c = *match++; len++; - if (special[c]) + switch (c) { + case 0: case '?': + case '\\': case '*': + case '[': return len; + } } }You are replacing a table-based check with a switch based, which might be substantially slower (depends on how often
cmp_name()is called).
Quoted 3 lines
Or faster. When the table gives a cache miss and has to be loaded from main memory, I am quite sure that 5 compares in a row are faster than the cache miss.
It is five compares time the length of the path being examined. The cache miss only occurs once per invocation even inthe worst case. Judging from where the code is invoked, cache misses should be rare which means the table is much faster.
As for the table being a micro-optimization, that still holds true
Quoted 4 lines
Actually, with the switch, the compiler is free to implement it with a table (and gcc usually does this, probably even using a substantially smaller table). The table-based check in contrast looks
It usually uses binary search. For gcc to create a lookup table you'll need a large number of case's.
Quoted 3 lines
like some kind of micro-optimization which makes the code IMHO more difficult to read, and which only would be justified with meassured improvements.
The table lookup *is* faster (meastured), but that doesn't make a big difference on the total CPU used. The muliple-case-per line thing (both versions, however makes is hard to read.
-- robin
dir.c +15 -5
@@ -423,12 +423,22 @@ static int cmp_name(const void *p1, const void *p2)*/ static int simple_length (const char *match) { -const char special[256 ] = { - [0 ] =1 , ['?' ] =1 , - ['\\' ] =1 , ['*' ] =1 , - ['[' ] =1 - };int len =-1 ; +static const char special[256 ] = { +1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,/* nul */ +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,1 ,0 ,0 ,0 ,0 ,0 ,/* * */ +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,/* ? */ + +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,1 ,1 ,0 ,0 ,0 /* [ \ */ + };for (;;) {unsigned char c = *match++;
Quoted 17 lines · Robin Rosenberg
The table lookup *is* faster (meastured), but that doesn't make a big difference on the total CPU used. The muliple-case-per line thing (both versions, however makes is hard to read.
-- robin
dir.c +0
-3@@ -423,12 +423,22 @@ static int cmp_name(const void *p1, const void *p2)*/ static int simple_length (const char *match) { -const char special[256 ] = { - [0 ] =1 , ['?' ] =1 , - ['\\' ] =1 , ['*' ] =1 ,
dir.c static int cmp_name(const void *p1, const void *p2) · line 433
Quoted 10 lines
dir.c +7
-2- ['['] = 1 - };int len =-1 ; +static const char special[256 ] = { +1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,/* nul */ +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 , +0 ,0 ,1 ,0 ,0 ,0 ,0 ,0 ,/* * */
I wonder if folding this into ctype.c::sane_ctype[] is an option...