{"thread":{"id":"7657","subject":"[PATCH] Correct dir.c to compile on Solaris 9","startedAt":"2007-04-15T04:33:28Z","lastAt":"2007-04-15T22:48:52Z","messageCount":5,"participants":["Shawn O. Pearce","Johannes Schindelin","Josef Weidendorfer","Robin Rosenberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39365","messageId":"20070415043328.GA17285@spearce.org","threadId":"7657","inReplyTo":null,"subject":"[PATCH] Correct dir.c to compile on Solaris 9","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-15T04:33:28Z","receivedAt":"2007-04-15T04:33:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The compiler on my Solaris 9 system doesn't understand\nthe array initialization syntax used here in dir.c.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n dir.c |   11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7426fde..038fd82 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -423,18 +423,17 @@ static int cmp_name(const void *p1, const void *p2)\n  */\n static int simple_length(const char *match)\n {\n-\tconst char special[256] = {\n-\t\t[0] = 1, ['?'] = 1,\n-\t\t['\\\\'] = 1, ['*'] = 1,\n-\t\t['['] = 1\n-\t};\n \tint len = -1;\n \n \tfor (;;) {\n \t\tunsigned char c = *match++;\n \t\tlen++;\n-\t\tif (special[c])\n+\t\tswitch (c) {\n+\t\tcase 0: case '?':\n+\t\tcase '\\\\': case '*':\n+\t\tcase '[':\n \t\t\treturn len;\n+\t\t}\n \t}\n }\n \n-- \n1.5.1.1.83.g2bfe3\n"},{"id":"39376","messageId":"Pine.LNX.4.64.0704151824240.18846@racer.site","threadId":"7657","inReplyTo":"20070415043328.GA17285@spearce.org","subject":"Re: [PATCH] Correct dir.c to compile on Solaris 9","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-15T16:25:48Z","receivedAt":"2007-04-15T16:25:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 15 Apr 2007, Shawn O. Pearce wrote:\n\n> The compiler on my Solaris 9 system doesn't understand\n> the array initialization syntax used here in dir.c.\n> \n> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n> ---\n>  dir.c |   11 +++++------\n>  1 files changed, 5 insertions(+), 6 deletions(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 7426fde..038fd82 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -423,18 +423,17 @@ static int cmp_name(const void *p1, const void *p2)\n>   */\n>  static int simple_length(const char *match)\n>  {\n> -\tconst char special[256] = {\n> -\t\t[0] = 1, ['?'] = 1,\n> -\t\t['\\\\'] = 1, ['*'] = 1,\n> -\t\t['['] = 1\n> -\t};\n>  \tint len = -1;\n>  \n>  \tfor (;;) {\n>  \t\tunsigned char c = *match++;\n>  \t\tlen++;\n> -\t\tif (special[c])\n> +\t\tswitch (c) {\n> +\t\tcase 0: case '?':\n> +\t\tcase '\\\\': case '*':\n> +\t\tcase '[':\n>  \t\t\treturn len;\n> +\t\t}\n>  \t}\n>  }\n\nYou are replacing a table-based check with a switch based, which might be \nsubstantially slower (depends on how often cmp_name() is called).\n\nMaybe there is another way to initialize the table (and make it static to \nbegin with)?\n\nCiao,\nDscho\n"},{"id":"39404","messageId":"200704152303.12435.Josef.Weidendorfer@gmx.de","threadId":"7657","inReplyTo":"Pine.LNX.4.64.0704151824240.18846@racer.site","subject":"Re: [PATCH] Correct dir.c to compile on Solaris 9","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2007-04-15T21:03:12Z","receivedAt":"2007-04-15T21:03:12Z","isPatch":true,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Sunday 15 April 2007, Johannes Schindelin wrote:\n> On Sun, 15 Apr 2007, Shawn O. Pearce wrote:\n> >  static int simple_length(const char *match)\n> >  {\n> > -\tconst char special[256] = {\n> > -\t\t[0] = 1, ['?'] = 1,\n> > -\t\t['\\\\'] = 1, ['*'] = 1,\n> > -\t\t['['] = 1\n> > -\t};\n> >  \tint len = -1;\n> >  \n> >  \tfor (;;) {\n> >  \t\tunsigned char c = *match++;\n> >  \t\tlen++;\n> > -\t\tif (special[c])\n> > +\t\tswitch (c) {\n> > +\t\tcase 0: case '?':\n> > +\t\tcase '\\\\': case '*':\n> > +\t\tcase '[':\n> >  \t\t\treturn len;\n> > +\t\t}\n> >  \t}\n> >  }\n> \n> You are replacing a table-based check with a switch based, which might be \n> substantially slower (depends on how often cmp_name() is called).\n\nOr faster. When the table gives a cache miss and has to be\nloaded from main memory, I am quite sure that 5 compares in a row are\nfaster than the cache miss.\n\nActually, with the switch, the compiler is free to implement it with a\ntable (and gcc usually does this, probably even using a substantially\nsmaller table). The table-based check in contrast looks\nlike some kind of micro-optimization which makes the code IMHO more\ndifficult to read, and which only would be justified with meassured\nimprovements.\n\nJosef\n"},{"id":"39410","messageId":"200704152354.55097.robin.rosenberg.lists@dewire.com","threadId":"7657","inReplyTo":"200704152303.12435.Josef.Weidendorfer@gmx.de","subject":"Re: [PATCH] Correct dir.c to compile on Solaris 9","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-04-15T21:54:53Z","receivedAt":"2007-04-15T21:54:53Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 15 april 2007 23:03 skrev Josef Weidendorfer:\n> On Sunday 15 April 2007, Johannes Schindelin wrote:\n> > On Sun, 15 Apr 2007, Shawn O. Pearce wrote:\n> > >  static int simple_length(const char *match)\n> > >  {\n> > > -\tconst char special[256] = {\n> > > -\t\t[0] = 1, ['?'] = 1,\n> > > -\t\t['\\\\'] = 1, ['*'] = 1,\n> > > -\t\t['['] = 1\n> > > -\t};\n> > >  \tint len = -1;\n> > >  \n> > >  \tfor (;;) {\n> > >  \t\tunsigned char c = *match++;\n> > >  \t\tlen++;\n> > > -\t\tif (special[c])\n> > > +\t\tswitch (c) {\n> > > +\t\tcase 0: case '?':\n> > > +\t\tcase '\\\\': case '*':\n> > > +\t\tcase '[':\n> > >  \t\t\treturn len;\n> > > +\t\t}\n> > >  \t}\n> > >  }\n> > \n> > You are replacing a table-based check with a switch based, which might be \n> > substantially slower (depends on how often cmp_name() is called).\n> \n> Or faster. When the table gives a cache miss and has to be\n> loaded from main memory, I am quite sure that 5 compares in a row are\n> faster than the cache miss.\nIt is five compares time the length of the path being examined. The cache miss\nonly occurs once per invocation even inthe worst case.  Judging from where\nthe code is invoked, cache misses should be rare which means the table is much\nfaster. \n\nAs for the table being a micro-optimization, that still holds true \n\n> \n> Actually, with the switch, the compiler is free to implement it with a\n> table (and gcc usually does this, probably even using a substantially\n> smaller table). The table-based check in contrast looks\nIt usually uses binary search. For gcc to create a lookup table you'll need\na large number of case's.\n> like some kind of micro-optimization which makes the code IMHO more\n> difficult to read, and which only would be justified with meassured\n> improvements.\n\nThe table lookup *is* faster (meastured), but that doesn't make a big difference\non the total CPU used. The muliple-case-per line thing (both versions, however makes is hard\nto read.  \n\n-- robin\n\ndiff --git a/dir.c b/dir.c\nindex 7426fde..0780f23 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -423,12 +423,22 @@ static int cmp_name(const void *p1, const void *p2)\n  */\n static int simple_length(const char *match)\n {\n-       const char special[256] = {\n-               [0] = 1, ['?'] = 1,\n-               ['\\\\'] = 1, ['*'] = 1,\n-               ['['] = 1\n-       };\n        int len = -1;\n+       static const char special[256] = {\n+               1,0,0,0,0,0,0,0, /* nul */\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,0,\n+               0,0,1,0,0,0,0,0, /* * */\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,1, /* ? */\n+\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,0,0,0,0,0,\n+               0,0,0,1,1,0,0,0  /* [ \\ */\n+       };\n\n        for (;;) {\n                unsigned char c = *match++;\n"},{"id":"39414","messageId":"7vbqhp2qzv.fsf@assigned-by-dhcp.cox.net","threadId":"7657","inReplyTo":"200704152354.55097.robin.rosenberg.lists@dewire.com","subject":"Re: [PATCH] Correct dir.c to compile on Solaris 9","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T22:48:52Z","receivedAt":"2007-04-15T22:48:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:\n\n> The table lookup *is* faster (meastured), but that doesn't make a big difference\n> on the total CPU used. The muliple-case-per line thing (both versions, however makes is hard\n> to read.  \n>\n> -- robin\n>\n> diff --git a/dir.c b/dir.c\n> index 7426fde..0780f23 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -423,12 +423,22 @@ static int cmp_name(const void *p1, const void *p2)\n>   */\n>  static int simple_length(const char *match)\n>  {\n> -       const char special[256] = {\n> -               [0] = 1, ['?'] = 1,\n> -               ['\\\\'] = 1, ['*'] = 1,\n> -               ['['] = 1\n> -       };\n>         int len = -1;\n> +       static const char special[256] = {\n> +               1,0,0,0,0,0,0,0, /* nul */\n> +               0,0,0,0,0,0,0,0,\n> +               0,0,0,0,0,0,0,0,\n> +               0,0,0,0,0,0,0,0,\n> +               0,0,0,0,0,0,0,0,\n> +               0,0,1,0,0,0,0,0, /* * */\n\nI wonder if folding this into ctype.c::sane_ctype[] is an\noption...\n"}]}