src/fccfg.c | 2 - src/fcinit.c | 4 +-- src/fcint.h | 10 ++++----- src/fclist.c | 10 ++++++++- src/fcname.c | 34 +++++++----------------------- src/fcpat.c | 65 ++++++++++++++++++++++------------------------------------- src/fcxml.c | 8 ++++--- 7 files changed, 55 insertions(+), 78 deletions(-) New commits: commit 4a060729a1466186d3be63ada344f43d66f937e5 Author: Akira TAGOH <akira@xxxxxxxxx> Date: Wed Mar 28 13:38:53 2012 +0900 fcpat: Increase the number of buckets in the shared string hash table This is a reasonably conservative increase in the number of buckets in the hash table to 251. After FcInit(), there are 240 shared strings in use on my system (from configuration files I assume). The hash value is stored in each link in the chains so comparison are actually not very expensive. This change should reduce the average length of chains by a factor of 8. With the reference counted strings, it should keep the average length of chains to about 2. The number of buckets is prime so as not to rely too much on the quality of the hash function. https://bugs.freedesktop.org/show_bug.cgi?id=17832#c5 Patch from Karl Tomlinson diff --git a/src/fcpat.c b/src/fcpat.c index 4b93c5e..54ec45c 100644 --- a/src/fcpat.c +++ b/src/fcpat.c @@ -1023,7 +1023,7 @@ bail0: return NULL; } -#define OBJECT_HASH_SIZE 31 +#define OBJECT_HASH_SIZE 251 static struct objectBucket { struct objectBucket *next; FcChar32 hash; commit d8dcff7b96b09748e6f1df9e4adc7ab0850d7b18 Author: Akira TAGOH <akira@xxxxxxxxx> Date: Wed Mar 28 13:37:15 2012 +0900 Bug 17832 - Memory leaks due to FcStrStaticName use for external patterns Use the reference-counted strings instead of the static strings Patch from Karl Tomlinson diff --git a/src/fccfg.c b/src/fccfg.c index 9395f74..bd1dc34 100644 --- a/src/fccfg.c +++ b/src/fccfg.c @@ -1009,7 +1009,7 @@ FcConfigEvaluate (FcPattern *p, FcExpr *e) case FcOpPlus: v.type = FcTypeString; str = FcStrPlus (vl.u.s, vr.u.s); - v.u.s = FcStrStaticName (str); + v.u.s = FcSharedStr (str); FcStrFree (str); if (!v.u.s) diff --git a/src/fcinit.c b/src/fcinit.c index b7966b6..abf64b5 100644 --- a/src/fcinit.c +++ b/src/fcinit.c @@ -139,7 +139,7 @@ FcFini (void) if (_fcConfig) FcConfigDestroy (_fcConfig); - FcPatternFini (); + FcObjectFini (); FcCacheFini (); if (FcDebug() & FC_DBG_MEMORY) FcMemReport (); @@ -221,7 +221,7 @@ static struct { { "vstack" }, { "attr" }, { "pstack" }, - { "staticstr" }, + { "sharedstr" }, }; static int FcAllocCount, FcAllocMem; diff --git a/src/fcint.h b/src/fcint.h index 8179195..56f77ef 100644 --- a/src/fcint.h +++ b/src/fcint.h @@ -103,7 +103,7 @@ #define FC_MEM_VSTACK 26 #define FC_MEM_ATTR 27 #define FC_MEM_PSTACK 28 -#define FC_MEM_STATICSTR 29 +#define FC_MEM_SHAREDSTR 29 #define FC_MEM_NUM 30 @@ -948,14 +948,14 @@ FcPatternObjectGetBool (const FcPattern *p, FcObject object, int n, FcBool *b); FcPrivate FcResult FcPatternObjectGetLangSet (const FcPattern *p, FcObject object, int n, FcLangSet **ls); -FcPrivate void -FcPatternFini (void); - FcPrivate FcBool FcPatternAppend (FcPattern *p, FcPattern *s); FcPrivate const FcChar8 * -FcStrStaticName (const FcChar8 *name); +FcSharedStr (const FcChar8 *name); + +FcPrivate FcBool +FcSharedStrFree (const FcChar8 *name); FcPrivate FcChar32 FcStringHash (const FcChar8 *s); diff --git a/src/fclist.c b/src/fclist.c index 9a84b5c..88025e9 100644 --- a/src/fclist.c +++ b/src/fclist.c @@ -67,13 +67,16 @@ FcObjectSetAdd (FcObjectSet *os, const char *object) low = 0; mid = 0; c = 1; - object = (char *)FcStrStaticName ((FcChar8 *)object); + object = (char *)FcSharedStr ((FcChar8 *)object); while (low <= high) { mid = (low + high) >> 1; c = os->objects[mid] - object; if (c == 0) + { + FcSharedStrFree ((FcChar8 *)object); return FcTrue; + } if (c < 0) low = mid + 1; else @@ -91,8 +94,13 @@ FcObjectSetAdd (FcObjectSet *os, const char *object) void FcObjectSetDestroy (FcObjectSet *os) { + int i; + if (os->objects) { + for (i = 0; i < os->nobject; i++) + FcSharedStrFree ((FcChar8 *)os->objects[i]); + FcMemFree (FC_MEM_OBJECTPTR, os->sobject * sizeof (const char *)); free ((void *) os->objects); } diff --git a/src/fcname.c b/src/fcname.c index 1b32b0f..d0b1ca8 100644 --- a/src/fcname.c +++ b/src/fcname.c @@ -572,9 +572,10 @@ FcNameBool (const FcChar8 *v, FcBool *result) } static FcValue -FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m) +FcNameConvert (FcType type, FcChar8 *string) { FcValue v; + FcMatrix m; v.type = type; switch (v.type) { @@ -583,7 +584,7 @@ FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m) v.u.i = atoi ((char *) string); break; case FcTypeString: - v.u.s = FcStrStaticName(string); + v.u.s = FcSharedStr (string); if (!v.u.s) v.type = FcTypeVoid; break; @@ -595,8 +596,8 @@ FcNameConvert (FcType type, FcChar8 *string, FcMatrix *m) v.u.d = strtod ((char *) string, 0); break; case FcTypeMatrix: - v.u.m = m; - sscanf ((char *) string, "%lg %lg %lg %lg", &m->xx, &m->xy, &m->yx, &m->yy); + sscanf ((char *) string, "%lg %lg %lg %lg", &m.xx, &m.xy, &m.yx, &m.yy); + v.u.m = FcMatrixCopy (&m); break; case FcTypeCharSet: v.u.c = FcNameParseCharSet (string); @@ -648,7 +649,6 @@ FcNameParse (const FcChar8 *name) FcChar8 *e; FcChar8 delim; FcValue v; - FcMatrix m; const FcObjectType *t; const FcConstant *c; @@ -699,31 +699,13 @@ FcNameParse (const FcChar8 *name) name = FcNameFindNext (name, ":,", save, &delim); if (t) { - v = FcNameConvert (t->type, save, &m); + v = FcNameConvert (t->type, save); if (!FcPatternAdd (pat, t->object, v, FcTrue)) { - switch (v.type) { - case FcTypeCharSet: - FcCharSetDestroy ((FcCharSet *) v.u.c); - break; - case FcTypeLangSet: - FcLangSetDestroy ((FcLangSet *) v.u.l); - break; - default: - break; - } + FcValueDestroy (v); goto bail2; } - switch (v.type) { - case FcTypeCharSet: - FcCharSetDestroy ((FcCharSet *) v.u.c); - break; - case FcTypeLangSet: - FcLangSetDestroy ((FcLangSet *) v.u.l); - break; - default: - break; - } + FcValueDestroy (v); } if (delim != ',') break; diff --git a/src/fcpat.c b/src/fcpat.c index 8f63659..4b93c5e 100644 --- a/src/fcpat.c +++ b/src/fcpat.c @@ -26,9 +26,6 @@ #include <string.h> #include <assert.h> -static FcBool -FcHashOwnsName(const FcChar8 *name); - FcPattern * FcPatternCreate (void) { @@ -50,7 +47,7 @@ FcValueDestroy (FcValue v) { switch (v.type) { case FcTypeString: - if (!FcHashOwnsName(v.u.s)) + if (!FcSharedStrFree (v.u.s)) FcStrFree ((FcChar8 *) v.u.s); break; case FcTypeMatrix: @@ -98,7 +95,7 @@ FcValueSave (FcValue v) { switch (v.type) { case FcTypeString: - v.u.s = FcStrStaticName (v.u.s); + v.u.s = FcSharedStr (v.u.s); if (!v.u.s) v.type = FcTypeVoid; break; @@ -131,7 +128,7 @@ FcValueListDestroy (FcValueListPtr l) { switch (l->value.type) { case FcTypeString: - if (!FcHashOwnsName((FcChar8 *)l->value.u.s)) + if (!FcSharedStrFree ((FcChar8 *)l->value.u.s)) FcStrFree ((FcChar8 *)l->value.u.s); break; case FcTypeMatrix: @@ -652,7 +649,7 @@ FcPatternObjectAddString (FcPattern *p, FcObject object, const FcChar8 *s) } v.type = FcTypeString; - v.u.s = FcStrStaticName(s); + v.u.s = s; return FcPatternObjectAdd (p, object, v, FcTrue); } @@ -1030,23 +1027,35 @@ bail0: static struct objectBucket { struct objectBucket *next; FcChar32 hash; + int ref_count; } *FcObjectBuckets[OBJECT_HASH_SIZE]; -static FcBool -FcHashOwnsName (const FcChar8 *name) +FcBool +FcSharedStrFree (const FcChar8 *name) { FcChar32 hash = FcStringHash (name); struct objectBucket **p; struct objectBucket *b; + int size; for (p = &FcObjectBuckets[hash % OBJECT_HASH_SIZE]; (b = *p); p = &(b->next)) if (b->hash == hash && ((char *)name == (char *) (b + 1))) + { + b->ref_count--; + if (!b->ref_count) + { + *p = b->next; + size = sizeof (struct objectBucket) + strlen ((char *)name) + 1; + FcMemFree (FC_MEM_SHAREDSTR, size + sizeof (int)); + free (b); + } return FcTrue; + } return FcFalse; } const FcChar8 * -FcStrStaticName (const FcChar8 *name) +FcSharedStr (const FcChar8 *name) { FcChar32 hash = FcStringHash (name); struct objectBucket **p; @@ -1055,7 +1064,10 @@ FcStrStaticName (const FcChar8 *name) for (p = &FcObjectBuckets[hash % OBJECT_HASH_SIZE]; (b = *p); p = &(b->next)) if (b->hash == hash && !strcmp ((char *)name, (char *) (b + 1))) + { + b->ref_count++; return (FcChar8 *) (b + 1); + } size = sizeof (struct objectBucket) + strlen ((char *)name) + 1; /* * workaround valgrind warning because glibc takes advantage of how it knows memory is @@ -1063,44 +1075,17 @@ FcStrStaticName (const FcChar8 *name) */ size = (size + 3) & ~3; b = malloc (size); - FcMemAlloc (FC_MEM_STATICSTR, size); + FcMemAlloc (FC_MEM_SHAREDSTR, size); if (!b) return NULL; b->next = 0; b->hash = hash; + b->ref_count = 1; strcpy ((char *) (b + 1), (char *)name); *p = b; return (FcChar8 *) (b + 1); } -static void -FcStrStaticNameFini (void) -{ - int i, size; - struct objectBucket *b, *next; - char *name; - - for (i = 0; i < OBJECT_HASH_SIZE; i++) - { - for (b = FcObjectBuckets[i]; b; b = next) - { - next = b->next; - name = (char *) (b + 1); - size = sizeof (struct objectBucket) + strlen (name) + 1; - FcMemFree (FC_MEM_STATICSTR, size + sizeof (int)); - free (b); - } - FcObjectBuckets[i] = 0; - } -} - -void -FcPatternFini (void) -{ - FcStrStaticNameFini (); - FcObjectFini (); -} - FcBool FcPatternSerializeAlloc (FcSerialize *serialize, const FcPattern *pat) { diff --git a/src/fcxml.c b/src/fcxml.c index ff30b7b..0fb82b6 100644 --- a/src/fcxml.c +++ b/src/fcxml.c @@ -104,7 +104,7 @@ FcExprCreateString (FcConfig *config, const FcChar8 *s) if (e) { e->op = FcOpString; - e->u.sval = FcStrStaticName (s); + e->u.sval = FcSharedStr (s); } return e; } @@ -176,7 +176,7 @@ FcExprCreateConst (FcConfig *config, const FcChar8 *constant) if (e) { e->op = FcOpConst; - e->u.constant = FcStrStaticName (constant); + e->u.constant = FcSharedStr (constant); } return e; } @@ -205,6 +205,7 @@ FcExprDestroy (FcExpr *e) case FcOpDouble: break; case FcOpString: + FcSharedStrFree (e->u.sval); break; case FcOpMatrix: FcMatrixFree (e->u.mval); @@ -222,6 +223,7 @@ FcExprDestroy (FcExpr *e) case FcOpField: break; case FcOpConst: + FcSharedStrFree (e->u.constant); break; case FcOpAssign: case FcOpAssignReplace: @@ -2134,7 +2136,7 @@ FcPopValue (FcConfigParse *parse) switch (vstack->tag) { case FcVStackString: - value.u.s = FcStrStaticName (vstack->u.string); + value.u.s = FcSharedStr (vstack->u.string); if (value.u.s) value.type = FcTypeString; break; _______________________________________________ Fontconfig mailing list Fontconfig@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/fontconfig