Attachment 139720 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), 8.57 KB, created by Alexander Larsson on 2018-05-23 17:35:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alexander Larsson

Created: 2018-05-23 17:35:10 UTC

Size: 8.57 KB

patch

obsolete

>From 01f5676763da9793c71b9ae4f72e2f4d418f6814 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   | 89 +++++++++++++++++++++++++++++++++++++++++++++------
> src/fcint.h   |  2 +-
> src/fclist.c  | 36 ---------------------
> src/fcmatch.c | 34 --------------------
> src/fcpat.c   | 10 ++++--
> 5 files changed, 87 insertions(+), 84 deletions(-)
>
>diff --git a/src/fccfg.c b/src/fccfg.c
>index e966e57..29afbba 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,88 @@ 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)
>+	    {
>+	      FcPatternElt *elts = FcPatternElts (font);
>+	      size_t i,j;
>+	      FcChar8 *data;
>+	      FcPattern *new_font;
>+	      FcPatternElt *new_elts;
>+	      FcValueList *new_value_list;
>+	      size_t new_path_len = strlen ((char *)relocated_font_file);
>+	      FcChar8 *new_path;
>+
>+	      data = FcCacheAllocate (cache,
>+				      sizeof (FcPattern) +
>+				      font->num * sizeof (FcPatternElt) +
>+				      sizeof (FcValueList) +
>+				      new_path_len + 1);
>+	      new_font = (FcPattern *)data;
>+	      data += sizeof (FcPattern);
>+	      new_elts = (FcPatternElt *)(data);
>+	      data += font->num * sizeof (FcPatternElt);
>+	      new_value_list = (FcValueList *)data;
>+	      data += sizeof (FcValueList);
>+	      new_path = data;
>+
>+	      *new_font = *font;
>+	      new_font->elts_offset = FcPtrToOffset (new_font, new_elts);
>+
>+	      /* Copy all but the FILE values from the cache */
>+	      for (i = 0, j = 0; i < font->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);
>+
>+	      font = new_font;
>+
>+	      free (relocated_font_file);
>+	    }
>+
> 	    if (FcFontSetAdd (config->fonts[set], font))
> 		nref++;
> 	}
>@@ -413,7 +482,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..bc409f5 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);
>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..8910f95 100644
>--- a/src/fcpat.c
>+++ b/src/fcpat.c
>@@ -382,16 +382,20 @@ FcPatternDestroy (FcPattern *p)
>     if (!p)
> 	return;
> 
>+    elts = FcPatternElts (p);
>+
>     if (FcRefIsConst (&p->ref))
>     {
>-	FcCacheObjectDereference (p);
>+	/* We use a value to find the cache, because the pattern
>+	   itself may be a cache allocation if we rewrote the path,
>+	   so the p pointer may not be in the cached region. */
>+	FcCacheObjectDereference (FcPatternEltValues(&elts[0]));
> 	return;
>     }
>-	
>+
>     if (FcRefDec (&p->ref) != 1)
> 	return;
> 
>-    elts = FcPatternElts (p);
>     for (i = 0; i < FcPatternObjectCount (p); i++)
> 	FcValueListDestroy (FcPatternEltValues(&elts[i]));
> 
>-- 
>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.