Re: strange code in gscanner.c

[Tim Janik <timj gtk org>]

On Wed, 27 Mar 2002, David Odin wrote:

>  While reading the code of gscanner.c (glib-2.0.0), I've found this

>               if ((value.v_identifier[0] == null_upper[0] ||
>                    value.v_identifier[0] == null_lower[0]) &&
>                   (value.v_identifier[1] == null_upper[1] ||
>                    value.v_identifier[1] == null_lower[1]) &&
>                   (value.v_identifier[2] == null_upper[2] ||
>                    value.v_identifier[2] == null_lower[2]) &&
>                   (value.v_identifier[3] == null_upper[3] ||
>                    value.v_identifier[3] == null_lower[3]))
[...]

> Is there any reason not to write it in this much simpler way:
>       if (token == G_TOKEN_IDENTIFIER &&
>           config->scan_identifier_NULL &&
>           strlen (value.v_identifier) == 4)
>         {
>           if (scanner->config->case_sensitive)
>             {
>               if (strcmp (value.v_identifier, "NULL") == 0)
>                 token = G_TOKEN_IDENTIFIER_NULL;
>             }
>           else
>             {
>               if (g_strcasecmp (value.v_identifier, NULL) == 0)
>                 token = G_TOKEN_IDENTIFIER_NULL;
>             }
>         }
> 
>    I think the later in much easier to read.

i think the direct comparison in question was written for efficiency
reasons. a quick test shows, that, depending on the value of
v_identifier, introducing strcmp() makes the current code from 1.5 to
2.5 times slower, introducing g_strcasecmp() makes the code around
7 times slower.
that's with glibc and -O2, note that glibc can already unfold certain
strcmp() calls, while the comparatively bad performance of g_strcasecmp()
is due to GLib rolling it's own loop.

> 
>    OK to commit?

nope ;)

> 
>     Regards,
> 
>            DindinX
> 

---
ciaoTJ