SUMMARY When the fontconfig dylib is used from a 32-bit process and then used from a 64-bit process (or vice versa), there is a significant delay as it attempts to rebuild the shared font caches. STEPS TO REPRODUCE 1. Remove any existing user font cache (you may also need to clean the system caches): rm -rf ~/.fontconfig 2. Compile two versions of the enclosed testfontconfig.c as follows: gcc -I/usr/X11/include -L/usr/X11/lib -lfontconfig -o testfontconfig32 -arch i386 testfontconfig.c gcc -I/usr/X11/include -L/usr/X11/lib -lfontconfig -o testfontconfig64 -arch x86_64 testfontconfig.c 3. Run the programs ./testfontconfig32 ./testfontconfig64 ./testfontconfig32 RESULTS Each run causes a significant delay. Inspection of ~/.fontconfig shows the caches have been rewritten on each run. EXPECTED RESULTS The third run should not result in any delay. REGRESSION Running the same architecture twice in a row doesn't result in a delay in-between e.g. ./testfontconfig32 ./testfontconfig32 NOTE The fontconfig as of version 2.6 detects the architecture suffix for cache files at build time and then hard-codes it into the executable. Therefore a Universal Binary build of fontconfig will have the same hard-coded architecture suffix for all architectures, and end up writing to the same set of cache files as well. Alternating between different architectural runs, fontconfig thinks the cache files written by the previous architecture are corrupt and then rewrites them. I've provided a patch (fontconfig.patch) which causes the architecture suffix for cache files to vary according to the currently building architecture. The cache files should then be distinct for the different architectures in a Universal Binary. I've also verified this fix builds and works. This bug is related to https://savannah.nongnu.org/bugs/index.php?23204, however while the freetype2 crash has been fixed as of OS X 10.5.6, the fontconfig cache file bug masked by the crash has not been fixed. I've also submitted to Apple via bug 6602251.
Created attachment 23101 [details] [review] Test file (testfontconfig.c) and patch to fix (fontconfig.patch).
Please attach the test file and the patch as separate, text, files.
Created attachment 23455 [details] [review] Patch to fix bug
Created attachment 23456 [details] Compile this to illustrate issue
(In reply to comment #2) > Please attach the test file and the patch as separate, text, files. > Done.
I'm not sure how that fixes the problem for you as you still end up with one architecture for the entire fat binary...
fontconfig.patch patches the fc-arch.c file so that the fc-arch program produces various #ifdef's for the different architectures. Then the rest of the fontconfig build consumes the output of fc-arch -- when gcc is passed different architectural flags through configure i.e. CFLAGS="-arch ppc -arch ppc64 -arch i386 -arch x86_64", it then transparently compiles the 4 architectures in 4 distinct passes then glues them together. Hope that explains it clear enough. The configure needs to be passed the CFLAGS in order to work, but that is the typical way most packages are built as universal binaries.
Does this issue still persist in 2.9.0?
This is not resolved by d1a0fca316ab8d9d61474028da54615e4d9f7540 because it checks values from configure checks which are based only on a single architecture. You can not rely on sizeof() checks or endian checks from configure as they do not apply to all archs. +#if defined(WORDS_BIGENDIAN) && WORDS_BIGENDIAN +# define FC_ARCH_ENDIAN "be" +#else /* !WORDS_BIGENDIAN */ +# define FC_ARCH_ENDIAN "le" +#endif + +#if SIZEOF_VOID_P == 4 +# if ALIGNOF_DOUBLE == 4 +# define FC_ARCH_SIZE_ALIGN "32d4" +# else /* ALIGNOF_DOUBLE != 4 */ +# define FC_ARCH_SIZE_ALIGN "32d8" +# endif +#else /* SIZEOF_VOID_P != 4 */ +# define FC_ARCH_SIZE_ALIGN "64" +#endif + +#ifdef ARCHITECTURE +# define ARCHITECTURE FC_ARCHITECTURE +#else +# define FC_ARCHITECTURE FC_ARCH_ENDIAN FC_ARCH_SIZE_ALIGN +#endif
Also, it looks like additional changes went in such that: /* config.h might override this */ #ifndef FC_ARCHITECTURE # define FC_ARCHITECTURE FC_ARCH_ENDIAN FC_ARCH_SIZE_ALIGN #endif So you determine the architecture by configure, so both i386 and x86_64 slices think they have the same signature.
I'm working on a patch for you...
Created attachment 58651 [details] [review] 0001-fcarch-Restrict-FC_ARCHITECTURE-usage-to-fcarch.h.patch
Created attachment 58652 [details] [review] 0002-fcarch-Check-for-architecture-type-at-runtime-rather.patch
(In reply to comment #9) > This is not resolved by d1a0fca316ab8d9d61474028da54615e4d9f7540 because it > checks values from configure checks which are based only on a single > architecture. You can not rely on sizeof() checks or endian checks from > configure as they do not apply to all archs. Since we are using autotools as part of the toolchain, I'd prefer fixing autotools rather than patching something out in fontconfig if its result isn't worth relying. (In reply to comment #10) > So you determine the architecture by configure, so both i386 and x86_64 slices > think they have the same signature. I can see different signature on both i386 and x86_64 on Linux at least though. I'm not familiar with Mac OS X and don't have any box for testing, so can you give me some examples how current code actually behaves on them and how looks like?
(In reply to comment #14) > (In reply to comment #9) > > This is not resolved by d1a0fca316ab8d9d61474028da54615e4d9f7540 because it > > checks values from configure checks which are based only on a single > > architecture. You can not rely on sizeof() checks or endian checks from > > configure as they do not apply to all archs. > > Since we are using autotools as part of the toolchain, I'd prefer fixing > autotools rather than patching something out in fontconfig if its result isn't > worth relying. You can't. This is a defect in autoconf *by design* It sets SIZEOF_VOID_P in config.h and there is no way that a single value will work for all architectures. > (In reply to comment #10) > > So you determine the architecture by configure, so both i386 and x86_64 slices > > think they have the same signature. > > I can see different signature on both i386 and x86_64 on Linux at least though. Linux does not support fat binaries, so that's not a good example. Your linux example would be akin to me building an i386 fcarch and a separate x86_64 fcarch. They'd both work. If you try to build a single fat binary, it won't > I'm not familiar with Mac OS X and don't have any box for testing, so can you > give me some examples how current code actually behaves on them and how looks > like? current code doesn't even build fat because it fails the FC_ASSERT_STATIC: FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (intptr_t)); For i386 sizeof(void *) is 4, for x86_64, sizeof(void *) is 8. configure sets SIZEOF_VOID_P to either 8 or 4 depending on if the build machine is 32bit or 64bit, and so the assertion will fail at build time for the other architecture. This is if we're lucky and building for differnet bit levels. If you try to build for i386 and ppc at the same time, you'll get both as either little endian or both as big endian.
(In reply to comment #15) > This is if we're lucky and building for differnet bit levels. If you try to > build for i386 and ppc at the same time, you'll get both as either little > endian or both as big endian. Actually, if you have a new enough autoconf, the endianness issue is worked around by autoconf, but SIZEOF_VOID_P and ALIGNOF_DOUBLE are still just based on a single architecture. It's not safe to make such checks at configure time (which is why I changed them to compile-time checks in the second patch). The changes I've provide do more than just support this particular case. They help to better future-proof the code.
If you want a patch that preserves FC_ARCHITECTURE as a string macro, I can do that instead, but it won't be future proof. That future-proofing is why I went with this approach.
Created attachment 58663 [details] [review] 0002-fcarch-Check-for-architecture-type-at-runtime-rather.patch
Created attachment 58664 [details] [review] 0002-fcarch-Check-for-architecture-type-at-runtime-rather.patch
Created attachment 58665 [details] [review] 0001-fcarch-Check-for-architecture-signature-at-compile-t.patch This one is more hacky and more fragile since we're still relying on the preprocessor for checks and can't use sizeof() or offsetof(), but it has less code change... I prefer the previous series for better flexibility going forward.
(In reply to comment #7) > Then the rest of the fontconfig build consumes the output of fc-arch -- when > gcc is passed different architectural flags through configure i.e. > CFLAGS="-arch ppc -arch ppc64 -arch i386 -arch x86_64", it then transparently > compiles the 4 architectures in 4 distinct passes then glues them together. Ah, got it. that clearly explains why you can't rerun configure script with the certain options to update the architecture information. I see why it needs to be addressed at the build time, really.
Patch looks fine.
to keep the backward compatibility (and not to keep the garbage cache files in the cache dir), I prefer the last one though, we could revisit this when the cache version is being bumped, which might be coming in not-so-distant future.
Fixed in 8cc44981.
Thinking about this again, since most of the trick is to fixup autoconf macros, I think we should move that to the bottom of config.h, using AH_BOTTOM. What do people think?
I seem to remember AH_BOTTOM doesn't actually work for this case, but I'm willing to try a patch.
Hmm, right. it seems not working properly. even though config.h.in has the correct macros as it's written in configure.in, it's badly replaced when config.h is generated. see: #ifdef __APPLE__ # include <machine/endian.h> # define SIZEOF_VOID_P 8 # define ALIGNOF_DOUBLE 8 # ifdef __LP64__ # define SIZEOF_VOID_P 8 # define ALIGNOF_DOUBLE 8 # else # define SIZEOF_VOID_P 8 # define ALIGNOF_DOUBLE 8 # endif #endif
(In reply to comment #27) > Hmm, right. it seems not working properly. even though config.h.in has the > correct macros as it's written in configure.in, it's badly replaced when > config.h is generated. see: > > #ifdef __APPLE__ > # include <machine/endian.h> > # define SIZEOF_VOID_P 8 > # define ALIGNOF_DOUBLE 8 > # ifdef __LP64__ > # define SIZEOF_VOID_P 8 > # define ALIGNOF_DOUBLE 8 > # else > # define SIZEOF_VOID_P 8 > # define ALIGNOF_DOUBLE 8 > # endif > #endif Autoconf manual suggests that we should put the custom code into a header and include it in AH_BOTTOM. I feel more confident that way. Cause, as it is right now, those macros are not reliable unless fcarch.h has been included...
Okay, proposed fix: http://cgit.freedesktop.org/~tagoh/fontconfig/commit/?h=bz20208 I'm not quite sure if it's really likely but this can be easily dropped once it's improved in autoconf.
Yeah, that looks fine. We can then easily add additional "fixups" to that as necessary.
Okay, merged into master. thanks!
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.