Re: Performance implications of GRegex structure

[mark mark mielke cc]

On Fri, Mar 16, 2007 at 02:18:23PM -0400, Owen Taylor wrote:
> On Fri, 2007-03-16 at 10:57 -0700, David Moffatt wrote:
> >   char *
> >   get_leading_digits(const char *str)
> >   {
> >      static GRegex *regex = NULL;
> >      char *result = NULL;
> > 
> >      if (!regex)
> >          regex = g_regex_new("^\\d+", 0, 0, NULL);
> > 
> >      if (g_regex_match(str, 0))
> >          result = g_regex_fetch(regex, 0);
> > 
> >      return result;
> >   }
> > 
> > That code bothers me a fair bit; not because so much because it's
> > not thread safe, but because it exhibits a pattern that is *inherently*
> > not thread safe or re-entrant....
> > 
> > Actually if you look at it carefully you realize it is thread safe and
> > depending on what you call the pattern it too is thread safe.  I think
> > this is a great way of doing things.  I work in embedded so we are very
> > sensitive to things like library global constructor creating regex.
> > That is overhead you pay whether or not you use the function.  That is
> > also overhead that you pay at launch time which for us is a big deal.
> 
> OK, maybe I'm just not looking at it right, but to me, it's not 
> thread safe two ways:
> 
>  A) The initializaton of the regex isn't thread safe. If multiple
>     threads race to construct the regex, there will be a memory
>     leak.
>  B) GRegex has internal state that is modified by g_regex_match()

Agree with both.

> None of the examples I shown used global constructors.

Correct. Overhead is only on first use.

Dave: Your detector is not sensitive enough I think... :-)

Cheers,
mark

-- 
mark mielke cc / markm ncf ca / markm nortel com     __________________________
.  .  _  ._  . .   .__    .  . ._. .__ .   . . .__  | Neighbourhood Coder
|\/| |_| |_| |/    |_     |\/|  |  |_  |   |/  |_   | 
|  | | | | \ | \   |__ .  |  | .|. |__ |__ | \ |__  | Ottawa, Ontario, Canada

  One ring to rule them all, one ring to find them, one ring to bring them all
                       and in the darkness bind them...

                           http://mark.mielke.cc/