Dynamically link against system fontconfig

[vially]

Statically linking the fontconfig library into the flutter app can cause (runtime) performance issues on systems where the statically linked fontconfig is using a different on-disk cache format version from the system fontconfig version. This is particularly relevant on systems with a large number of fonts installed: flutter/flutter#118911.

This pull-request fixes that by replacing the statically linked fontconfig version with dynamic linking to the system version. For what it's worth, by default Skia is also dynamically linking against system fontconfig.

Warning: This requires a similar change to the Flutter engine (flutter/engine#40725).

Underlying root issue

On a typical Linux desktop distribution¹, the package manager automatically updates the system fontconfig cache (e.g.: /var/cache/fontconfig) when system fonts are installed.

This means that applications using a version of fontconfig with the same on-disk cache format as the system fontconfig should be able to use the system cache (and therefore have reasonably fast font discovery).

However, when the application and system fontconfig versions do not match, the app is unable to use the system font cache so it has to re-index all the system fonts (which is a significantly slower operation).

In addition to that, in an attempt to speed-up subsequent launches of the app, the fontconfig library persists the system font cache to disk on application startup. Since user-space apps typically do not have write access to the system font cache directory (e.g.: /var/cache/fontconfig), the fontconfig library ends up writing the cache to the user fontconfig cache (e.g.: ~/.cache/fontconfig) which ends up slowing the application startup even futher (due to the extra IO).

This part is a bit unclear as to why it happens, but it turns out fontconfig does not attempt to read the user cache on the next app startup. So the whole process starts over again on every app launch: system font cache not found => fonts re-indexed => font cache written to user cache (which is never to be read again).

Benchmarks

Note

These tests were performed on an ArchLinux system with approximately 6000 system fonts installed (ttf-google-fonts-git) using version 2.14.2 of fontconfig (FC_CACHE_VERSION: 8).

The following graph shows the startup times (time to first frame as reported by flutter run --profile) for ten consecutive runs using statically linked fontconfig versus dynamically linked fontconfig. The average startup time for statically linked fontconfig was 2186 ms versus 368 ms for dynamically linked.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read the Flutter Style Guide recently, and have followed its advice.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

1. This has only been tested on ArchLinux but I would expect other Linux distributions to work in a similar way. ↩

[chinmaygarde]

I am supportive of this change but waiting on hearing back on the original issue regarding the impact of this for third party embedders.

[cbracken]

Overall, I expect this will be entirely fine/positive for the GTK embedder. @robert-ancell Are there any implications for snap packaging? I imagine that Flutter snaps already depend on some form of GTK dependencies that bring in fontconfig transitively. Not sure if fontconfig is part of the base system or packaged as a snap; if a snap, should we update Flutter snap packaging to depend on it directly?

@chinmaygarde have your reached out to third-party embedders via the chat room/mailing lists? In terms of known third-party customers we've worked with directly, I've got contact info for a few and I'm sure the PMs have more too.

[cbracken]

Another option would be to produce both a static and dynamically linked build and allow people to opt-in to the dynamic one (or perhaps make it default and provide instructions for opt-ing back out).

[chinmaygarde]

Paging @robert-ancell for the question on Snap packaging. You're right that we could do the static one for engine dylib version and do the dynamic version of the Linux embedder.

[robert-ancell]

I think that a snapped Flutter app is expected to use the GNOME platform snap (e.g. gnome-42-2204) to access fontconfig dynamically. @kenvandine there's no case where you'd want to use a statically linked version, right?

[kenvandine]

Indeed, we want to use the dynamically linked fontconfig for snaps. So a +1 from me on this change

…

On Sun, May 14, 2023, 7:35 PM Robert Ancell ***@***.***> wrote: I think that a snapped Flutter app is expected to use the GNOME platform snap (e.g. gnome-42-2204) to access fontconfig dynamically. @kenvandine <https://github.com/kenvandine> there's no case where you'd want to use a statically linked version, right? — Reply to this email directly, view it on GitHub <#701 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANMBNKILX2XHGIS77SMA3XGFT3RANCNFSM6AAAAAAWLDBZGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cbracken]

lgtm from the Linux embedder point of view.

@chinmaygarde thoughts on whether we want to continue to offer a build target for the statically linked variant for third-party embedded users?

[chinmaygarde]

thoughts on whether we want to continue to offer a build target for the statically linked variant for third-party embedded users

I think we should keep the existing dylibs the same since there are strong stability guarantees. The GTK embedder can link it dynamically.

[chinmaygarde]

@cbracken May I land this please?

[auto-submit]

auto label is removed for flutter/buildroot, pr: 701, due to This PR has not met approval requirements for merging. You are not a member of flutter-hackers and need 1 more review(s) in order to merge this PR.

• Merge guidelines: You need at least one approved review if you are already part of flutter-hackers or two member reviews if you are not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

[cbracken]

Yep go for it. Since it's coming from someone who isn't on the team, it needs one more lgtm.

[robert-ancell]

LGTM