This commit introduces a bad performance regression. Every time fc-cache is run, it runs through the directories twice. The first time, it sees valid caches and does nothing. The second time, it always rebuilds.
This adds approximately 90s (compared to 5s) to the launch time of XQuartz. It also causes the initial cache build process (done during a post install script) to take twice as long:
AFTER COMMIT (with valid cache):
$ sudo /opt/X11/bin/fc-cache -v -s
/opt/X11/share/fonts: skipping, existing cache is valid: 0 fonts, 10 dirs
/opt/X11/share/fonts/100dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/opt/X11/share/fonts/75dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/opt/X11/share/fonts/OTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/opt/X11/share/fonts/Speedo: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/TTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/opt/X11/share/fonts/Type1: skipping, existing cache is valid: 29 fonts, 0 dirs
/opt/X11/share/fonts/cyrillic: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/encodings: skipping, existing cache is valid: 0 fonts, 1 dirs
/opt/X11/share/fonts/encodings/large: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/misc: skipping, existing cache is valid: 59 fonts, 0 dirs
/opt/X11/share/fonts/util: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts: skipping, existing cache is valid: 0 fonts, 10 dirs
/usr/X11R6/lib/X11/fonts/100dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/75dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/OTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/Speedo: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/TTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/Type1: skipping, existing cache is valid: 29 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/cyrillic: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/encodings: skipping, existing cache is valid: 0 fonts, 1 dirs
/usr/X11R6/lib/X11/fonts/encodings/large: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/misc: skipping, existing cache is valid: 59 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/util: skipping, existing cache is valid: 0 fonts, 0 dirs
/.local/share/fonts: skipping, no such directory
/System/Library/Fonts: skipping, existing cache is valid: 121 fonts, 0 dirs
/Library/Fonts: skipping, existing cache is valid: 505 fonts, 1 dirs
/Library/Fonts/Microsoft: skipping, existing cache is valid: 207 fonts, 0 dirs
/opt/X11/share/fonts: caching, new cache contents: 0 fonts, 10 dirs
/usr/X11R6/lib/X11/fonts: caching, new cache contents: 0 fonts, 10 dirs
/.local/share/fonts: skipping, no such directory
/System/Library/Fonts: caching, new cache contents: 121 fonts, 0 dirs
/Library/Fonts: caching, new cache contents: 505 fonts, 1 dirs
/opt/X11/var/cache/fontconfig: cleaning cache directory
/.cache/fontconfig: not cleaning non-existent cache directory
/opt/X11/bin/fc-cache: succeeded
---
BEFORE COMMIT (with valid cache):
$ sudo /opt/X11/bin/fc-cache -v -s
/opt/X11/share/fonts: skipping, existing cache is valid: 0 fonts, 10 dirs
/opt/X11/share/fonts/100dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/opt/X11/share/fonts/75dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/opt/X11/share/fonts/OTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/opt/X11/share/fonts/Speedo: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/TTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/opt/X11/share/fonts/Type1: skipping, existing cache is valid: 29 fonts, 0 dirs
/opt/X11/share/fonts/cyrillic: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/encodings: skipping, existing cache is valid: 0 fonts, 1 dirs
/opt/X11/share/fonts/encodings/large: skipping, existing cache is valid: 0 fonts, 0 dirs
/opt/X11/share/fonts/misc: skipping, existing cache is valid: 59 fonts, 0 dirs
/opt/X11/share/fonts/util: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts: skipping, existing cache is valid: 0 fonts, 10 dirs
/usr/X11R6/lib/X11/fonts/100dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/75dpi: skipping, existing cache is valid: 398 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/OTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/Speedo: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/TTF: skipping, existing cache is valid: 23 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/Type1: skipping, existing cache is valid: 29 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/cyrillic: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/encodings: skipping, existing cache is valid: 0 fonts, 1 dirs
/usr/X11R6/lib/X11/fonts/encodings/large: skipping, existing cache is valid: 0 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/misc: skipping, existing cache is valid: 59 fonts, 0 dirs
/usr/X11R6/lib/X11/fonts/util: skipping, existing cache is valid: 0 fonts, 0 dirs
/.local/share/fonts: skipping, no such directory
/System/Library/Fonts: skipping, existing cache is valid: 121 fonts, 0 dirs
/Library/Fonts: skipping, existing cache is valid: 505 fonts, 1 dirs
/Library/Fonts/Microsoft: skipping, existing cache is valid: 207 fonts, 0 dirs
/opt/X11/var/cache/fontconfig: cleaning cache directory
/.cache/fontconfig: not cleaning non-existent cache directory
/opt/X11/bin/fc-cache: succeeded
---
0203055520206028eecee5d261887cdc91500e15 is the first bad commit
commit 0203055520206028eecee5d261887cdc91500e15
Author: Akira TAGOH <akira@xxxxxxxxx>
Date: Wed Oct 2 16:34:34 2013 +0900
Workaround the race condition issue on updating cache
:040000 040000 c46bb6dbd52a09a3bd732c4acce7ddbd86d30c80 0b2bb37c2f87d6b45b6998e1a1dc4c8f311fd490 M fc-cache
:040000 040000 f3d04196f0f9ffa9bb33f335db50ed80e2657e38 cac1d2ed883d783b72296349749919d6fd7284ae M fontconfig
:040000 040000 a49c45834272aa5d3e7e8d4341f7737f11e99f59 aa1c56d3ee818e5e68df2dfda74f10938fe84ede M src
---
git bisect start
# good: [02db01ac22318b2e296e6e1fd9664cac1ae66442] Bump version to 2.10.2
git bisect good 02db01ac22318b2e296e6e1fd9664cac1ae66442
# bad: [767108aa1327cf0156dfc6f024dbc8fb783ae067] Correct DTD
git bisect bad 767108aa1327cf0156dfc6f024dbc8fb783ae067
# good: [671bcb34e23ed03b1f564af35560db81f8b12b96] Better fix for 2fe5ddfd
git bisect good 671bcb34e23ed03b1f564af35560db81f8b12b96
# good: [38ab7ab2fbd83c0c62e4b78302b5fe89da0cb79e] Fix a incompatible pointer warning on NetBSD
git bisect good 38ab7ab2fbd83c0c62e4b78302b5fe89da0cb79e
# good: [d2bb1a8381ba50dce79a487cd82087ca57fdcb68] Bump version to 2.10.95
git bisect good d2bb1a8381ba50dce79a487cd82087ca57fdcb68
# bad: [0203055520206028eecee5d261887cdc91500e15] Workaround the race condition issue on updating cache
git bisect bad 0203055520206028eecee5d261887cdc91500e15
# good: [643f8088f0d51107e58d142df47124efec6afab1] Further changes to 30-metric-aliases.conf
git bisect good 643f8088f0d51107e58d142df47124efec6afab1
# good: [102864d0dba46c99b22c912454c1f58731287405] Add the description of -q option to the man page
git bisect good 102864d0dba46c99b22c912454c1f58731287405
# good: [96c5f3cf0ff57e7bbb08cc1e0e78ed0542096484] clean up
git bisect good 96c5f3cf0ff57e7bbb08cc1e0e78ed0542096484
# good: [9161ed1e4a3f4afaee6dbcfc0b84a279ad99b397] Add the relative path for <include> to fonts.conf if the parent path is same to fonts.conf
git bisect good 9161ed1e4a3f4afaee6dbcfc0b84a279ad99b397
# first bad commit: [0203055520206028eecee5d261887cdc91500e15] Workaround the race condition issue on updating cache
On Oct 2, 2013, at 0:35, Akira TAGOH <tagoh@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> fc-cache/fc-cache.c | 62 +++++++++++++++++++++++++++---------------------
> fontconfig/fontconfig.h | 3 ++
> src/fcstr.c | 6 ++++
> 3 files changed, 45 insertions(+), 26 deletions(-)
>
> New commits:
> commit 0203055520206028eecee5d261887cdc91500e15
> Author: Akira TAGOH <akira@xxxxxxxxx>
> Date: Wed Oct 2 16:34:34 2013 +0900
>
> Workaround the race condition issue on updating cache
>
> diff --git a/fc-cache/fc-cache.c b/fc-cache/fc-cache.c
> index aeb0af2..af7ba6d 100644
> --- a/fc-cache/fc-cache.c
> +++ b/fc-cache/fc-cache.c
> @@ -118,7 +118,7 @@ usage (char *program, int error)
> static FcStrSet *processed_dirs;
>
> static int
> -scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, FcBool verbose, int *changed)
> +scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, FcBool verbose, FcBool recursive, int *changed)
> {
> int ret = 0;
> const FcChar8 *dir;
> @@ -141,7 +141,7 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force,
> fflush (stdout);
> }
>
> - if (FcStrSetMember (processed_dirs, dir))
> + if (recursive && FcStrSetMember (processed_dirs, dir))
> {
> if (verbose)
> printf ("skipping, looped directory detected\n");
> @@ -213,32 +213,37 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force,
> ret++;
> }
> }
> -
> - subdirs = FcStrSetCreate ();
> - if (!subdirs)
> +
> + if (recursive)
> {
> - fprintf (stderr, "%s: Can't create subdir set\n", dir);
> - ret++;
> - FcDirCacheUnload (cache);
> - continue;
> - }
> - for (i = 0; i < FcCacheNumSubdir (cache); i++)
> - FcStrSetAdd (subdirs, FcCacheSubdir (cache, i));
> + subdirs = FcStrSetCreate ();
> + if (!subdirs)
> + {
> + fprintf (stderr, "%s: Can't create subdir set\n", dir);
> + ret++;
> + FcDirCacheUnload (cache);
> + continue;
> + }
> + for (i = 0; i < FcCacheNumSubdir (cache); i++)
> + FcStrSetAdd (subdirs, FcCacheSubdir (cache, i));
>
> - FcDirCacheUnload (cache);
> + FcDirCacheUnload (cache);
>
> - sublist = FcStrListCreate (subdirs);
> - FcStrSetDestroy (subdirs);
> - if (!sublist)
> - {
> - fprintf (stderr, "%s: Can't create subdir list\n", dir);
> - ret++;
> - continue;
> + sublist = FcStrListCreate (subdirs);
> + FcStrSetDestroy (subdirs);
> + if (!sublist)
> + {
> + fprintf (stderr, "%s: Can't create subdir list\n", dir);
> + ret++;
> + continue;
> + }
> + FcStrSetAdd (processed_dirs, dir);
> + ret += scanDirs (sublist, config, force, really_force, verbose, recursive, changed);
> + FcStrListDone (sublist);
> }
> - FcStrSetAdd (processed_dirs, dir);
> - ret += scanDirs (sublist, config, force, really_force, verbose, changed);
> + else
> + FcDirCacheUnload (cache);
> }
> - FcStrListDone (list);
> return ret;
> }
>
> @@ -366,7 +371,11 @@ main (int argc, char **argv)
> }
>
> changed = 0;
> - ret = scanDirs (list, config, force, really_force, verbose, &changed);
> + ret = scanDirs (list, config, force, really_force, verbose, FcTrue, &changed);
> + /* Update the directory cache again to avoid the race condition as much as possible */
> + FcStrListFirst (list);
> + ret += scanDirs (list, config, FcTrue, really_force, verbose, FcFalse, &changed);
> + FcStrListDone (list);
>
> /*
> * Try to create CACHEDIR.TAG anyway.
> @@ -379,6 +388,8 @@ main (int argc, char **argv)
>
> cleanCacheDirectories (config, verbose);
>
> + FcConfigDestroy (config);
> + FcFini ();
> /*
> * Now we need to sleep a second (or two, to be extra sure), to make
> * sure that timestamps for changes after this run of fc-cache are later
> @@ -386,8 +397,7 @@ main (int argc, char **argv)
> * sleep(3) can't be interrupted by a signal here -- this isn't in the
> * library, and there aren't any signals flying around here.
> */
> - FcConfigDestroy (config);
> - FcFini ();
> + /* the resolution of mtime on FAT is 2 seconds */
> if (changed)
> sleep (2);
> if (verbose)
> diff --git a/fontconfig/fontconfig.h b/fontconfig/fontconfig.h
> index 58912f5..e588579 100644
> --- a/fontconfig/fontconfig.h
> +++ b/fontconfig/fontconfig.h
> @@ -974,6 +974,9 @@ FcStrSetDestroy (FcStrSet *set);
> FcPublic FcStrList *
> FcStrListCreate (FcStrSet *set);
>
> +FcPublic void
> +FcStrListFirst (FcStrList *list);
> +
> FcPublic FcChar8 *
> FcStrListNext (FcStrList *list);
>
> diff --git a/src/fcstr.c b/src/fcstr.c
> index 3a32031..5707172 100644
> --- a/src/fcstr.c
> +++ b/src/fcstr.c
> @@ -1374,6 +1374,12 @@ FcStrListCreate (FcStrSet *set)
> return list;
> }
>
> +void
> +FcStrListFirst (FcStrList *list)
> +{
> + list->n = 0;
> +}
> +
> FcChar8 *
> FcStrListNext (FcStrList *list)
> {
> _______________________________________________
> Fontconfig mailing list
> Fontconfig@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/fontconfig
>
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Fontconfig mailing list Fontconfig@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/fontconfig
