[Fontconfig] [PATCH] Cache most recent regcomp() call in _FcStrRegexCmp().

[Akira TAGOH]

Okay, that makes sense. will do.

On Thu, May 2, 2013 at 6:51 AM, Behdad Esfahbod <behdad at behdad.org> wrote:
> On 13-05-01 05:29 PM, Nick Alcock wrote:
>>
>> Under the circumstances, it is unfortunate that _FcStrRegexCmp() is called so
>> very often by fontconfig.  On my system (with ~5000 fonts, and a stripped-down
>> fonts.conf), mapping a new Emacs frame makes twelve calls to FcFontMatch()...
>> and those twelve calls proceed to call _FcStrRegexCmp() 175171 times, with a
>> call to regcomp() every time!  This then proceeds to invoke malloc() on the
>> order of three million times.  It is fairly easy to turn this into a
>> pathological slowdown at runtime.  My Emacsen are not small (arena sizes of a
>> gigabyte-plus are common, with a highly fragmented heap), and in that situation
>> those three million malloc() and free() calls can easily consume in excess of
>> fifty seconds (though on a fresh startup it takes 'only' a quarter of a second
>> or so).
>
> It's really strange that that function is called at all.  Can you figure out
> why that is?  Does your configuration have anything involving the "file" element?
>
> Akira, regardless, I think we should remove the Regex and replace it with Glob
> matching that is already in fccfg.c.
>
> I know you want to extend regex to other elements, but for files I think globs
> are just fine.
>
> behdad
>
>> Since each call to FcFontMatch() in this pathological case (and probably in most
>> other cases) calls _FcStrRegexCmp() with a constant argument, a trivial cache
>> consisting of a single static variable suffices to reduce the number of calls to
>> regcomp() to... eleven.  (The precise method used may need a little rethinking
>> for thread-safety, but the general principle, of reducing regcomp() calls via
>> some variety of caching, appears to be essential.)
>>
>> ---
>>  src/fcstr.c | 31 +++++++++++++++++++++++--------
>>  1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> [Resent for the second time: if this doesn't get through I will consider
>>  the fontconfig list broken...]
>>
>> diff --git a/src/fcstr.c b/src/fcstr.c
>> index 339a346..fa4560d 100644
>> --- a/src/fcstr.c
>> +++ b/src/fcstr.c
>> @@ -247,19 +247,32 @@ static FcBool
>>  _FcStrRegexCmp (const FcChar8 *s, const FcChar8 *regex, int cflags, int eflags)
>>  {
>>      int ret = -1;
>> -    regex_t reg;
>> +    static regex_t reg;
>> +    static FcChar8 *last;
>>
>> -    if ((ret = regcomp (&reg, (const char *)regex, cflags)) != 0)
>> +    if (!regex || !last || FcStrCmp (regex, last) != 0)
>>      {
>> -     if (FcDebug () & FC_DBG_MATCHV)
>> +     if (last != NULL)
>>       {
>> -         char buf[512];
>> +         regfree (&reg);
>> +         last = NULL;
>> +     }
>>
>> -         regerror (ret, &reg, buf, 512);
>> -         printf("Regexp compile error: %s\n", buf);
>> +     if ((ret = regcomp (&reg, (const char *)regex, cflags)) != 0)
>> +     {
>> +         if (FcDebug () & FC_DBG_MATCHV)
>> +         {
>> +             char buf[512];
>> +
>> +             regerror (ret, &reg, buf, 512);
>> +             printf("Regexp compile error: %s\n", buf);
>> +         }
>> +         return FcFalse;
>>       }
>> -     return FcFalse;
>> +     FcFree (last);
>> +     last = FcStrdup (regex);
>>      }
>> +
>>      ret = regexec (&reg, (const char *)s, 0, NULL, eflags);
>>      if (ret != 0)
>>      {
>> @@ -271,7 +284,9 @@ _FcStrRegexCmp (const FcChar8 *s, const FcChar8 *regex, int cflags, int eflags)
>>           printf("Regexp exec error: %s\n", buf);
>>       }
>>      }
>> -    regfree (&reg);
>> +
>> +    if (last == NULL)  /* only on OOM */
>> +     regfree (&reg);
>>
>>      return ret == 0 ? FcTrue : FcFalse;
>>  }
>>
>
> --
> behdad
> http://behdad.org/
> _______________________________________________
> Fontconfig mailing list
> Fontconfig at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/fontconfig



-- 
Akira TAGOH