GNOME.org
 

[pango] Bug 689342 - Lifetime of patterns in pattern_hash may use too much memory

commit 7ed3cb89923c376d8b30ae3f977ab9e1a231e430
Author: Behdad Esfahbod <behdad behdad org>
Date:   Thu Dec 20 00:51:55 2012 -0500

    Bug 689342 - Lifetime of patterns in pattern_hash may use too much memory
    
    Refcount cached patterns.

 pango/pangofc-fontmap.c |  207 ++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 161 insertions(+), 46 deletions(-)
---
diff --git a/pango/pangofc-fontmap.c b/pango/pangofc-fontmap.c
index 78ce5d7..017fe44 100644
--- a/pango/pangofc-fontmap.c
+++ b/pango/pangofc-fontmap.c
@@ -44,7 +44,10 @@
  * - All FcPattern's referenced by any object in the fontmap are uniquified
  *   and cached in the fontmap.  This both speeds lookups based on patterns
  *   faster, and saves memory.  This is handled by fontmap->priv->pattern_hash.
- *   The patterns are cached indefinitely.
+ *   The patterns are refcounted via PangoFcUPattern and removed from
+ *   pattern_hash when the reference count reaches zero. This avoids an ever
+ *   growing pattern_hash when a very large number of patterns are used within
+ *   the same font map (think an infinitely varied zoom factor on the text).
  *
  * - The results of a FcFontSort() are used to populate fontsets.  However,
  *   FcFontSort() relies on the search pattern only, which includes the font
@@ -89,6 +92,7 @@ typedef struct _PangoFcFontFaceData PangoFcFontFaceData;
 typedef struct _PangoFcFace         PangoFcFace;
 typedef struct _PangoFcFamily       PangoFcFamily;
 typedef struct _PangoFcFindFuncInfo PangoFcFindFuncInfo;
+typedef struct _PangoFcUPattern     PangoFcUPattern;
 typedef struct _PangoFcPatterns     PangoFcPatterns;
 typedef struct _PangoFcFontset      PangoFcFontset;
 
@@ -111,7 +115,7 @@ struct _PangoFcFontMapPrivate
 
   GHashTable *font_hash;	/* Maps PangoFcFontKey -> PangoFcFont */
 
-  GHashTable *patterns_hash;	/* Maps FcPattern -> PangoFcPatterns */
+  GHashTable *patterns_hash;	/* Maps PangoFcUPattern -> PangoFcPatterns */
 
   /* pattern_hash is used to make sure we only store one copy of
    * each identical pattern. (Speeds up lookup).
@@ -215,13 +219,24 @@ static gboolean           pango_fc_fontset_key_equal (const PangoFcFontsetKey *k
 static void               pango_fc_font_key_init     (PangoFcFontKey       *key,
 						      PangoFcFontMap       *fcfontmap,
 						      PangoFcFontsetKey    *fontset_key,
-						      FcPattern            *pattern);
+						      PangoFcUPattern      *pattern);
 static PangoFcFontKey    *pango_fc_font_key_copy     (const PangoFcFontKey *key);
 static void               pango_fc_font_key_free     (PangoFcFontKey       *key);
 static guint              pango_fc_font_key_hash     (const PangoFcFontKey *key);
 static gboolean           pango_fc_font_key_equal    (const PangoFcFontKey *key_a,
 						      const PangoFcFontKey *key_b);
 
+static guint              pango_fc_upattern_hash     (const PangoFcUPattern *upattern);
+static gboolean           pango_fc_upattern_equal    (const PangoFcUPattern *upattern_a,
+						      const PangoFcUPattern *upattern_b);
+static void               pango_fc_upattern_free     (PangoFcUPattern       *upattern);
+static PangoFcUPattern   *pango_fc_upattern_create   (FcPattern             *pattern);
+static PangoFcUPattern   *pango_fc_upattern_init_key (PangoFcUPattern       *upattern,
+						      FcPattern             *pattern);
+static PangoFcUPattern   *pango_fc_upattern_ref      (PangoFcUPattern       *upattern);
+static void               pango_fc_upattern_unref    (PangoFcUPattern       *upattern,
+						      PangoFcFontMap        *fcfontmap);
+
 static PangoFcPatterns *pango_fc_patterns_new   (FcPattern       *pat,
 						 PangoFcFontMap  *fontmap);
 static PangoFcPatterns *pango_fc_patterns_ref   (PangoFcPatterns *pats);
@@ -229,8 +244,8 @@ static void             pango_fc_patterns_unref (PangoFcPatterns *pats);
 static FcPattern       *pango_fc_patterns_get_pattern      (PangoFcPatterns *pats);
 static FcPattern       *pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i);
 
-static FcPattern *uniquify_pattern (PangoFcFontMap *fcfontmap,
-				    FcPattern      *pattern);
+static PangoFcUPattern *uniquify_pattern (PangoFcFontMap *fcfontmap,
+					  FcPattern      *pattern);
 
 static gpointer
 get_gravity_class (void)
@@ -324,6 +339,87 @@ get_scaled_size (PangoFcFontMap             *fcfontmap,
 }
 
 
+/*
+ * PangoFcUPattern
+ */
+
+struct _PangoFcUPattern {
+  FcPattern *pattern; /* only element that matters for equality in pattern_hash */
+  guint ref_count;    /* references from outside the pattern_hash */
+};
+
+static guint
+pango_fc_upattern_hash (const PangoFcUPattern *upattern)
+{
+  return FcPatternHash(upattern->pattern);
+}
+
+static gboolean
+pango_fc_upattern_equal (const PangoFcUPattern *upattern_a, const PangoFcUPattern *upattern_b)
+{
+  return FcPatternEqual(upattern_a->pattern, upattern_b->pattern);
+}
+
+static void
+pango_fc_upattern_free (PangoFcUPattern *upattern)
+{
+  if (upattern->ref_count == 0)
+    {
+      FcPatternDestroy(upattern->pattern);
+      g_slice_free(PangoFcUPattern, upattern);
+    }
+}
+
+static PangoFcUPattern *
+pango_fc_upattern_create(FcPattern *pattern)
+{
+  PangoFcUPattern *upat;
+
+  upat = g_slice_new(PangoFcUPattern);
+
+  FcPatternReference(pattern);
+
+  upat->pattern = pattern;
+  upat->ref_count = 1;
+
+  return upat;
+}
+
+static PangoFcUPattern *
+pango_fc_upattern_init_key (PangoFcUPattern *upattern, FcPattern *pattern)
+{
+  upattern->pattern = pattern;
+  upattern->ref_count = 0;
+
+  return upattern;
+}
+
+static PangoFcUPattern *
+pango_fc_upattern_ref (PangoFcUPattern *upattern)
+{
+  g_return_val_if_fail (upattern->ref_count > 0, NULL);
+
+  upattern->ref_count++;
+
+  return upattern;
+}
+
+static void
+pango_fc_upattern_unref (PangoFcUPattern *upattern, PangoFcFontMap *fcfontmap)
+{
+  g_return_if_fail (upattern->ref_count > 0);
+
+  if ( --upattern->ref_count > 0 )
+    return;
+
+  /* Only remove from pattern_hash if we are really in it (and not just the same
+   * pattern). This is not necessarily the case after a cache_clear() call. */
+  if (fcfontmap && fcfontmap->priv->pattern_hash &&
+      upattern == g_hash_table_lookup(fcfontmap->priv->pattern_hash, upattern))
+    g_hash_table_remove (fcfontmap->priv->pattern_hash, upattern);
+  else
+    pango_fc_upattern_free(upattern);
+}
 
 struct _PangoFcFontsetKey {
   PangoFcFontMap *fontmap;
@@ -337,7 +433,7 @@ struct _PangoFcFontsetKey {
 
 struct _PangoFcFontKey {
   PangoFcFontMap *fontmap;
-  FcPattern *pattern;
+  PangoFcUPattern *upattern;
   PangoMatrix matrix;
   gpointer context_key;
 };
@@ -544,7 +640,7 @@ static gboolean
 pango_fc_font_key_equal (const PangoFcFontKey *key_a,
 			 const PangoFcFontKey *key_b)
 {
-  if (key_a->pattern == key_b->pattern &&
+  if (key_a->upattern == key_b->upattern &&
       0 == memcmp (&key_a->matrix, &key_b->matrix, 4 * sizeof (double)))
     {
       if (key_a->context_key && key_b->context_key)
@@ -570,14 +666,14 @@ pango_fc_font_key_hash (const PangoFcFontKey *key)
       hash ^= PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_hash (key->fontmap,
 									    key->context_key);
 
-    return (hash ^ GPOINTER_TO_UINT (key->pattern));
+    return (hash ^ GPOINTER_TO_UINT (key->upattern));
 }
 
 static void
 pango_fc_font_key_free (PangoFcFontKey *key)
 {
-  if (key->pattern)
-    FcPatternDestroy (key->pattern);
+  if (key->upattern)
+    pango_fc_upattern_unref(key->upattern, key->fontmap);
 
   if (key->context_key)
     PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_free (key->fontmap,
@@ -592,8 +688,8 @@ pango_fc_font_key_copy (const PangoFcFontKey *old)
   PangoFcFontKey *key = g_slice_new (PangoFcFontKey);
 
   key->fontmap = old->fontmap;
-  FcPatternReference (old->pattern);
-  key->pattern = old->pattern;
+  pango_fc_upattern_ref(old->upattern);
+  key->upattern = old->upattern;
   key->matrix = old->matrix;
   if (old->context_key)
     key->context_key = PANGO_FC_FONT_MAP_GET_CLASS (key->fontmap)->context_key_copy (key->fontmap,
@@ -608,10 +704,10 @@ static void
 pango_fc_font_key_init (PangoFcFontKey    *key,
 			PangoFcFontMap    *fcfontmap,
 			PangoFcFontsetKey *fontset_key,
-			FcPattern         *pattern)
+			PangoFcUPattern   *upattern)
 {
   key->fontmap = fcfontmap;
-  key->pattern = pattern;
+  key->upattern = upattern;
   key->matrix = *pango_fc_fontset_key_get_matrix (fontset_key);
   key->context_key = pango_fc_fontset_key_get_context_key (fontset_key);
 }
@@ -631,7 +727,7 @@ pango_fc_font_key_init (PangoFcFontKey    *key,
 const FcPattern *
 pango_fc_font_key_get_pattern (const PangoFcFontKey *key)
 {
-  return key->pattern;
+  return key->upattern->pattern;
 }
 
 /**
@@ -676,7 +772,7 @@ struct _PangoFcPatterns {
 
   PangoFcFontMap *fontmap;
 
-  FcPattern *pattern;
+  PangoFcUPattern *upattern;
   FcPattern *match;
   FcFontSet *fontset;
 };
@@ -685,22 +781,25 @@ static PangoFcPatterns *
 pango_fc_patterns_new (FcPattern *pat, PangoFcFontMap *fontmap)
 {
   PangoFcPatterns *pats;
+  PangoFcUPattern *upat;
 
-  pat = uniquify_pattern (fontmap, pat);
-  pats = g_hash_table_lookup (fontmap->priv->patterns_hash, pat);
+  upat = uniquify_pattern (fontmap, pat);
+  pats = g_hash_table_lookup (fontmap->priv->patterns_hash, upat);
   if (pats)
-    return pango_fc_patterns_ref (pats);
+    {
+      pango_fc_upattern_unref(upat, fontmap);
+      return pango_fc_patterns_ref (pats);
+    }
 
   pats = g_slice_new0 (PangoFcPatterns);
 
   pats->fontmap = fontmap;
 
   pats->ref_count = 1;
-  FcPatternReference (pat);
-  pats->pattern = pat;
+  pats->upattern = upat;
 
   g_hash_table_insert (fontmap->priv->patterns_hash,
-		       pats->pattern, pats);
+		       pats->upattern, pats);
 
   return pats;
 }
@@ -728,12 +827,12 @@ pango_fc_patterns_unref (PangoFcPatterns *pats)
   /* Only remove from fontmap hash if we are in it.  This is not necessarily
    * the case after a cache_clear() call. */
   if (pats->fontmap->priv->patterns_hash &&
-      pats == g_hash_table_lookup (pats->fontmap->priv->patterns_hash, pats->pattern))
+      pats == g_hash_table_lookup (pats->fontmap->priv->patterns_hash, pats->upattern))
     g_hash_table_remove (pats->fontmap->priv->patterns_hash,
-			 pats->pattern);
+			 pats->upattern);
 
-  if (pats->pattern)
-    FcPatternDestroy (pats->pattern);
+  if (pats->upattern)
+    pango_fc_upattern_unref (pats->upattern, pats->fontmap);
 
   if (pats->match)
     FcPatternDestroy (pats->match);
@@ -747,7 +846,7 @@ pango_fc_patterns_unref (PangoFcPatterns *pats)
 static FcPattern *
 pango_fc_patterns_get_pattern (PangoFcPatterns *pats)
 {
-  return pats->pattern;
+  return pats->upattern->pattern;
 }
 
 static FcPattern *
@@ -758,7 +857,7 @@ pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i)
       FcResult result;
       if (!pats->match && !pats->fontset)
         {
-	  pats->match = FcFontMatch (NULL, pats->pattern, &result);
+	  pats->match = FcFontMatch (NULL, pats->upattern->pattern, &result);
 	}
 
       if (pats->match)
@@ -769,7 +868,7 @@ pango_fc_patterns_get_font_pattern (PangoFcPatterns *pats, int i)
       if (!pats->fontset)
         {
 	  FcResult result;
-	  pats->fontset = FcFontSort (NULL, pats->pattern, FcTrue, NULL, &result);
+	  pats->fontset = FcFontSort (NULL, pats->upattern->pattern, FcTrue, NULL, &result);
 	  if (pats->match)
 	    {
 	      FcPatternDestroy (pats->match);
@@ -1045,9 +1144,9 @@ pango_fc_font_map_init (PangoFcFontMap *fcfontmap)
 
   priv->patterns_hash = g_hash_table_new (NULL, NULL);
 
-  priv->pattern_hash = g_hash_table_new_full ((GHashFunc) FcPatternHash,
-					      (GEqualFunc) FcPatternEqual,
-					      (GDestroyNotify) FcPatternDestroy,
+  priv->pattern_hash = g_hash_table_new_full ((GHashFunc) pango_fc_upattern_hash,
+					      (GEqualFunc) pango_fc_upattern_equal,
+					      (GDestroyNotify) pango_fc_upattern_free,
 					      NULL);
 
   priv->font_face_data_hash = g_hash_table_new_full ((GHashFunc)pango_fc_font_face_data_hash,
@@ -1482,23 +1581,27 @@ pango_fc_make_pattern (const  PangoFontDescription *description,
   return pattern;
 }
 
-static FcPattern *
+/* The returned PangoFcUPattern should be unreferenced via
+ * pango_fc_upattern_unref() when done with it. */
+static PangoFcUPattern *
 uniquify_pattern (PangoFcFontMap *fcfontmap,
 		  FcPattern      *pattern)
 {
   PangoFcFontMapPrivate *priv = fcfontmap->priv;
-  FcPattern *old_pattern;
+  PangoFcUPattern upat_key;
+  PangoFcUPattern *upattern;
 
-  old_pattern = g_hash_table_lookup (priv->pattern_hash, pattern);
-  if (old_pattern)
+  upattern = g_hash_table_lookup (priv->pattern_hash,
+				  pango_fc_upattern_init_key(&upat_key, pattern));
+  if ( upattern )
     {
-      return old_pattern;
+      return pango_fc_upattern_ref(upattern);
     }
   else
     {
-      FcPatternReference (pattern);
-      g_hash_table_insert (priv->pattern_hash, pattern, pattern);
-      return pattern;
+      upattern = pango_fc_upattern_create(pattern);
+      g_hash_table_insert (priv->pattern_hash, upattern, upattern);
+      return upattern;
     }
 }
 
@@ -1509,6 +1612,7 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
 {
   PangoFcFontMapClass *class;
   PangoFcFontMapPrivate *priv = fcfontmap->priv;
+  PangoFcUPattern *umatch;
   FcPattern *pattern;
   PangoFcFont *fcfont;
   PangoFcFontKey key;
@@ -1516,13 +1620,16 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
   if (priv->closed)
     return NULL;
 
-  match = uniquify_pattern (fcfontmap, match);
+  umatch = uniquify_pattern (fcfontmap, match);
 
-  pango_fc_font_key_init (&key, fcfontmap, fontset_key, match);
+  pango_fc_font_key_init (&key, fcfontmap, fontset_key, umatch);
 
   fcfont = g_hash_table_lookup (priv->font_hash, &key);
   if (fcfont)
-    return g_object_ref (fcfont);
+    {
+      pango_fc_upattern_unref(umatch, fcfontmap);
+      return g_object_ref (fcfont);
+    }
 
   class = PANGO_FC_FONT_MAP_GET_CLASS (fcfontmap);
 
@@ -1534,6 +1641,7 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
     {
       const PangoMatrix *pango_matrix = pango_fc_fontset_key_get_matrix (fontset_key);
       FcMatrix fc_matrix, *fc_matrix_val;
+      PangoFcUPattern *upattern;
       int i;
 
       /* Fontconfig has the Y axis pointing up, Pango, down.
@@ -1543,7 +1651,7 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
       fc_matrix.yx = - pango_matrix->yx;
       fc_matrix.yy = pango_matrix->yy;
 
-      pattern = FcPatternDuplicate (match);
+      pattern = FcPatternDuplicate (umatch->pattern);
 
       for (i = 0; FcPatternGetMatrix (pattern, FC_MATRIX, i, &fc_matrix_val) == FcResultMatch; i++)
 	FcMatrixMultiply (&fc_matrix, &fc_matrix, fc_matrix_val);
@@ -1551,13 +1659,18 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
       FcPatternDel (pattern, FC_MATRIX);
       FcPatternAddMatrix (pattern, FC_MATRIX, &fc_matrix);
 
-      fcfont = class->new_font (fcfontmap, uniquify_pattern (fcfontmap, pattern));
+      upattern = uniquify_pattern (fcfontmap, pattern);
+      fcfont = class->new_font (fcfontmap, upattern->pattern);
+      pango_fc_upattern_unref(upattern, fcfontmap);
 
       FcPatternDestroy (pattern);
     }
 
   if (!fcfont)
-    return NULL;
+    {
+      pango_fc_upattern_unref(umatch, fcfontmap);
+      return NULL;
+    }
 
   fcfont->matrix = key.matrix;
   /* In case the backend didn't set the fontmap */
@@ -1569,6 +1682,8 @@ pango_fc_font_map_new_font (PangoFcFontMap    *fcfontmap,
   /* cache it on fontmap */
   pango_fc_font_map_add (fcfontmap, &key, fcfont);
 
+  pango_fc_upattern_unref(umatch, fcfontmap);
+
   return (PangoFont *)fcfont;
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]