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 at tagoh.org>
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 at tagoh.org>
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;