Attachment 139730 Details for Bug 106618 – Rewrite paths in caches earlier

[patch] Rewrite paths in caches earlier

0002-Cache-Rewrite-relocated-paths-in-earlier.patch (text/plain), 9.51 KB, created by Alexander Larsson on 2018-05-24 07:13:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alexander Larsson

Created: 2018-05-24 07:13:15 UTC

Size: 9.51 KB

patch

obsolete

>From 021983209e4223753579735a212ce1149805b51f Mon Sep 17 00:00:00 2001
>From: Alexander Larsson <alexl@redhat.com>
>Date: Wed, 23 May 2018 15:15:33 +0200
>Subject: [PATCH 2/3] Cache: Rewrite relocated paths in earlier
>
>This changes the rewriting of the FC_FILE values for relocated caches to an earlier stage
>while reading the cache. This is better, because it means all APIs will report the
>rewritten paths, not just the once that use the list apis.
>
>We do this by detecting the relocated case and duplicating the FcPattern and FcPatternElm
>in an cache allocation (which will die with the cache) and then reusing the FcValueLists
>from the cache.
>
>This means that in the rewritten case we will use some more memory, but not the full
>size of the cache. In a test here I had 800k of relocated caches, but ~200k of wasted
>on duplicating the objects.
>
>This should fix https://bugs.freedesktop.org/show_bug.cgi?id=106618
>---
> src/fccfg.c   | 44 +++++++++++++++++++++++--------
> src/fcint.h   |  5 +++-
> src/fclist.c  | 36 --------------------------
> src/fcmatch.c | 34 ------------------------
> src/fcpat.c   | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 5 files changed, 106 insertions(+), 84 deletions(-)
>
>diff --git a/src/fccfg.c b/src/fccfg.c
>index e966e57..ffe189a 100644
>--- a/src/fccfg.c
>+++ b/src/fccfg.c
>@@ -324,11 +324,15 @@ FcConfigDestroy (FcConfig *config)
> 
> FcBool
> FcConfigAddCache (FcConfig *config, FcCache *cache,
>-		  FcSetName set, FcStrSet *dirSet)
>+		  FcSetName set, FcStrSet *dirSet, FcChar8 *forDir)
> {
>     FcFontSet	*fs;
>     intptr_t	*dirs;
>     int		i;
>+    FcBool      relocated = FcFalse;
>+
>+    if (strcmp ((char *)FcCacheDir(cache), (char *)forDir) != 0)
>+      relocated = FcTrue;
> 
>     /*
>      * Add fonts
>@@ -342,23 +346,43 @@ FcConfigAddCache (FcConfig *config, FcCache *cache,
> 	{
> 	    FcPattern	*font = FcFontSetFont (fs, i);
> 	    FcChar8	*font_file;
>+	    FcChar8	*relocated_font_file = NULL;
> 
>-	    /*
>-	     * Check to see if font is banned by filename
>-	     */
> 	    if (FcPatternObjectGetString (font, FC_FILE_OBJECT,
>-					  0, &font_file) == FcResultMatch &&
>-		!FcConfigAcceptFilename (config, font_file))
>+					  0, &font_file) == FcResultMatch)
> 	    {
>-		continue;
>+		if (relocated)
>+		  {
>+		    FcChar8 *slash = FcStrLastSlash (font_file);
>+		    relocated_font_file = FcStrBuildFilename (forDir, slash + 1, NULL);
>+		    font_file = relocated_font_file;
>+		  }
>+
>+		/*
>+		 * Check to see if font is banned by filename
>+		 */
>+		if (!FcConfigAcceptFilename (config, font_file))
>+		{
>+		    free (relocated_font_file);
>+		    continue;
>+		}
> 	    }
>-		
>+
> 	    /*
> 	     * Check to see if font is banned by pattern
> 	     */
> 	    if (!FcConfigAcceptFont (config, font))
>+	    {
>+		free (relocated_font_file);
> 		continue;
>-		
>+	    }
>+
>+	    if (relocated_font_file)
>+	    {
>+	      font = FcPatternCacheRewriteFile (font, cache, relocated_font_file);
>+	      free (relocated_font_file);
>+	    }
>+
> 	    if (FcFontSetAdd (config->fonts[set], font))
> 		nref++;
> 	}
>@@ -413,7 +437,7 @@ FcConfigAddDirList (FcConfig *config, FcSetName set, FcStrSet *dirSet)
> 	cache = FcDirCacheRead (dir, FcFalse, config);
> 	if (!cache)
> 	    continue;
>-	FcConfigAddCache (config, cache, set, dirSet);
>+	FcConfigAddCache (config, cache, set, dirSet, dir);
> 	FcDirCacheUnload (cache);
>     }
>     FcStrListDone (dirlist);
>diff --git a/src/fcint.h b/src/fcint.h
>index fe21583..966559f 100644
>--- a/src/fcint.h
>+++ b/src/fcint.h
>@@ -712,7 +712,7 @@ FcConfigModifiedTime (FcConfig *config);
> 
> FcPrivate FcBool
> FcConfigAddCache (FcConfig *config, FcCache *cache,
>-		  FcSetName set, FcStrSet *dirSet);
>+		  FcSetName set, FcStrSet *dirSet, FcChar8 *forDir);
> 
> FcPrivate FcRuleSet *
> FcRuleSetCreate (const FcChar8 *name);
>@@ -1163,6 +1163,9 @@ FcPatternIterGetObjectId (const FcPattern *pat, FcPatternIter *iter);
> FcPrivate FcValueListPtr
> FcPatternIterGetValues (const FcPattern *pat, FcPatternIter *iter);
> 
>+FcPrivate FcPattern *
>+FcPatternCacheRewriteFile (const FcPattern *pat, FcCache *cache, const FcChar8 *relocated_font_file);
>+
> FcPrivate FcChar32
> FcStringHash (const FcChar8 *s);
> 
>diff --git a/src/fclist.c b/src/fclist.c
>index 5f92a72..d7e8fc0 100644
>--- a/src/fclist.c
>+++ b/src/fclist.c
>@@ -448,41 +448,6 @@ FcListAppend (FcListHashTable	*table,
> 	e = FcPatternObjectFindElt (font, FcObjectFromName (os->objects[o]));
> 	if (e)
> 	{
>-	    if (FcRefIsConst (&font->ref) && !strcmp (os->objects[o], FC_FILE))
>-	    {
>-		FcChar8 *dir, *alias;
>-		FcConfig *config = FcConfigGetCurrent (); /* FIXME: this may need to be exported as API? */
>-
>-		for (v = FcPatternEltValues (e); v->value.type != FcTypeString; v = FcValueListNext (v));
>-		if (!v)
>-		    goto bail2;
>-		dir = FcStrDirname (FcValueString (&v->value));
>-		if (FcHashTableFind (config->alias_table, dir, (void **) &alias))
>-		{
>-		    FcChar8 *base = FcStrBasename (FcValueString (&v->value));
>-		    FcChar8 *s = FcStrBuildFilename (alias, base, NULL);
>-		    FcValue vv;
>-
>-		    FcStrFree (alias);
>-		    FcStrFree (base);
>-		    vv.type = FcTypeString;
>-		    vv.u.s = s;
>-		    if (!FcPatternAdd (bucket->pattern,
>-				       os->objects[o],
>-				       FcValueCanonicalize (&vv),
>-				       FcTrue))
>-		    {
>-			FcStrFree (s);
>-			FcStrFree (dir);
>-			goto bail2;
>-		    }
>-		    FcStrFree (s);
>-		    FcStrFree (dir);
>-		    goto bail3;
>-		}
>-		else
>-		    FcStrFree (dir);
>-	    }
> 	    for (v = FcPatternEltValues(e), idx = 0; v;
> 		 v = FcValueListNext(v), ++idx)
> 	    {
>@@ -491,7 +456,6 @@ FcListAppend (FcListHashTable	*table,
> 				   FcValueCanonicalize(&v->value), defidx != idx))
> 		    goto bail2;
> 	    }
>-	  bail3:;
> 	}
>     }
>     *prev = bucket;
>diff --git a/src/fcmatch.c b/src/fcmatch.c
>index 62f8e58..4b3f599 100644
>--- a/src/fcmatch.c
>+++ b/src/fcmatch.c
>@@ -682,43 +682,9 @@ FcFontRenderPrepare (FcConfig	    *config,
> 	}
> 	else
> 	{
>-	    if (FcRefIsConst (&font->ref) && fe->object == FC_FILE_OBJECT)
>-	    {
>-		FcValueListPtr l = FcPatternEltValues (fe);
>-		FcChar8 *dir, *alias;
>-
>-		while (l->value.type != FcTypeString)
>-		    l = FcValueListNext (l);
>-		if (!l)
>-		    goto bail0;
>-		dir = FcStrDirname (FcValueString (&l->value));
>-		if (!config)
>-		    config = FcConfigGetCurrent ();
>-		if (config && FcHashTableFind (config->alias_table, dir, (void **) &alias))
>-		{
>-		    FcChar8 *base = FcStrBasename (FcValueString (&l->value));
>-		    FcChar8 *s = FcStrBuildFilename (alias, base, NULL);
>-		    FcValue v;
>-
>-		    FcStrFree (alias);
>-		    FcStrFree (base);
>-		    v.type = FcTypeString;
>-		    v.u.s = s;
>-		    FcPatternObjectAddWithBinding (new, fe->object,
>-						   FcValueCanonicalize (&v),
>-						   l->binding,
>-						   FcTrue);
>-		    FcStrFree (s);
>-		    FcStrFree (dir);
>-		    goto bail0;
>-		}
>-		else
>-		    FcStrFree (dir);
>-	    }
> 	    FcPatternObjectListAdd (new, fe->object,
> 				    FcValueListDuplicate (FcPatternEltValues (fe)),
> 				    FcTrue);
>-	  bail0:;
> 	}
>     }
>     for (i = 0; i < pat->num; i++)
>diff --git a/src/fcpat.c b/src/fcpat.c
>index eb534c3..0861b8f 100644
>--- a/src/fcpat.c
>+++ b/src/fcpat.c
>@@ -373,6 +373,71 @@ FcValueListHash (FcValueListPtr l)
>     return hash;
> }
> 
>+static void *
>+FcPatternGetCacheObject (FcPattern *p)
>+{
>+  /* We use a value to find the cache, instead of the FcPattern object
>+   * because the pattern itself may be a cache allocation if we rewrote the path,
>+   * so the p may not be in the cached region. */
>+  return FcPatternEltValues(&FcPatternElts (p)[0]);
>+}
>+
>+FcPattern *
>+FcPatternCacheRewriteFile (const FcPattern *p,
>+                           FcCache *cache,
>+                           const FcChar8 *relocated_font_file)
>+{
>+    FcPatternElt *elts = FcPatternElts (p);
>+    size_t i,j;
>+    FcChar8 *data;
>+    FcPattern *new_p;
>+    FcPatternElt *new_elts;
>+    FcValueList *new_value_list;
>+    size_t new_path_len = strlen ((char *)relocated_font_file);
>+    FcChar8 *new_path;
>+
>+    /* Allocate space for the patter, the PatternElt headers and
>+     * the FC_FILE FcValueList and path that will be freed with the
>+     * cache */
>+    data = FcCacheAllocate (cache,
>+			    sizeof (FcPattern) +
>+			    p->num * sizeof (FcPatternElt) +
>+			    sizeof (FcValueList) +
>+			    new_path_len + 1);
>+
>+    new_p = (FcPattern *)data;
>+    data += sizeof (FcPattern);
>+    new_elts = (FcPatternElt *)(data);
>+    data += p->num * sizeof (FcPatternElt);
>+    new_value_list = (FcValueList *)data;
>+    data += sizeof (FcValueList);
>+    new_path = data;
>+
>+    *new_p = *p;
>+    new_p->elts_offset = FcPtrToOffset (new_p, new_elts);
>+
>+    /* Copy all but the FILE values from the cache */
>+    for (i = 0, j = 0; i < p->num; i++)
>+    {
>+	FcPatternElt *elt = &elts[i];
>+	new_elts[j].object = elt->object;
>+	if (elt->object != FC_FILE_OBJECT)
>+	    new_elts[j++].values = FcPatternEltValues(elt);
>+	else
>+	    new_elts[j++].values = new_value_list;
>+    }
>+
>+    new_value_list->next = NULL;
>+    new_value_list->value.type = FcTypeString;
>+    new_value_list->value.u.s = new_path;
>+    new_value_list->binding = FcValueBindingWeak;
>+
>+    /* Add rewritten path at the end */
>+    strcpy ((char *)new_path, (char *)relocated_font_file);
>+
>+    return new_p;
>+}
>+
> void
> FcPatternDestroy (FcPattern *p)
> {
>@@ -384,10 +449,10 @@ FcPatternDestroy (FcPattern *p)
> 
>     if (FcRefIsConst (&p->ref))
>     {
>-	FcCacheObjectDereference (p);
>+	FcCacheObjectDereference (FcPatternGetCacheObject(p));
> 	return;
>     }
>-	
>+
>     if (FcRefDec (&p->ref) != 1)
> 	return;
> 
>@@ -1164,7 +1229,7 @@ FcPatternReference (FcPattern *p)
>     if (!FcRefIsConst (&p->ref))
> 	FcRefInc (&p->ref);
>     else
>-	FcCacheObjectReference (p);
>+	FcCacheObjectReference (FcPatternGetCacheObject(p));
> }
> 
> FcPattern *
>-- 
>2.17.0
>

Actions: View | Splinter Review

Attachments on bug 106618: 139711 | 139712 | 139713 | 139720 | 139730 | 139731

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.