GNOME Bugzilla – Bug 104495
Cache fontsets
Last modified: 2004-12-22 21:47:04 UTC
Here is a patch that caches fontsets indexed by patterns. I am attaching a benchmark that creates 50000 layouts and measures their sizes. The patch reduces the time to run that benchmark by about 40% (from ~13.5 seconds to ~8.0 seconds on my machine). I don't think the benchmark is unrealistic, because aGtkTreeView with many rows in it will actually create a lot of small layouts and measure their size. It may be overkill to cache all fontsets ever created. Presumably, most applications only uses a few fonts overall, so limiting the cache to, say, 8 fontsets may be enough.
Created attachment 13843 [details] The benchmark
Created attachment 13844 [details] [review] The patch that adds caching of fontsets
Created attachment 13845 [details] [review] New patch that doesn't leak the fontset cache
I really dislike extending the amount of Caching going on in Pango. It already caches a _lot_; the more caching going on, the more memory we waste, and the more it's hard to tell what's really going on performance wise. Do you know what is slow about creating the fontset? is it looking up the cached fonts that compose the fontset? (The loop in pango_fc_font_map_load_fontset())
Created attachment 13877 [details] profile of the benchmark
I just attached a scrrenshot of memprof showing a profile of the benchmark. As far as I can see, the fonts are *removed* from the cache before they are added to the fontset. Then, when the fontset is destroyed, all the fonts in it are also destroyed. Maybe this is the real problem.
Something is funny about this profile .. look at pango_xft_font_dispose. The total should equal the sum of the children and self, right?
But pango_xft_font_dispose() is not expanded, so you can't tell how much time is spent in its children. pango_xft_font_map_cache_add() and pango_xft_font_get_type() are children of g_object_last_unref(), not of pango_xft_font_dispose. The number in the "Non-recursive" column should equal self plus the sum of the children's non-recursive numbers, though (which they do in this profile, as far as I can tell). The sum of the numbers in the Total column doesn't mean anything, because that would include the same samples over and over.
Ah, sorry, misread it and thought it was expanded.
Putting on 1.2.2 at least until we have a good diagnosis on whether the font cache isn't working, or whether digging the fonts out of the cache and putting them back is just taking a lot of time. (Note that artificial benchmarks can be a _little_ deceptive here, since in real situations, the fonts often are ref'ed elsewhere, though it's not that odd to have a different font for, say, a GtkTextView, than the rest of your app.)
Created attachment 14206 [details] Benchmark that measures time spent doing treeview validation
I just attached a benchmark that measures time spent in tree view validation for a tree view with 20000 rows. On my machine, the fontset caching makes the validation time go down from 58.8s to 26.9s, a pretty big speedup.
Created attachment 14207 [details] [review] Patch that caches only one fontset
The patch I just attached caches just one fontset, the most recently used one. In addition it prints out whether a load was a cache hit or a cache miss. For the treeview validation benchmark, this patch achieves roughly the same speed up (provided you turn off the hit/miss spew). I also tested this one-fontset-cache with Gnumeric starting up. I got 662 hits and 12 misses, a hit rate of 98%.
> Putting on 1.2.2 at least until we have a good diagnosis > on whether the font cache isn't working, or whether digging > the fonts out of the cache and putting them back is just > taking a lot of time. I have investigated this, and the font cache seems to work fine; all the overhead comes from cache bookkeeping and object disposing. Some highlights: 32% in pango_xft_font_dispose() (14 of these in pango_xft_font_map_cache_add()) 6% in pango_fontset_simple_finalize() 8% in pango_fontset_simple_append() 22% in pango_fc_font_map_new_font() (11 in pango_fc_font_map_cache_remove()) With a small fontset cache, maybe the font cache isn't really necessary?
Created attachment 14226 [details] [review] Cache one fontmap per context
Yet another variant... This one caches one fontmap per context and saves about 10% of gnumeric's drawing time. (It also fixes a ref count problem in pango_context_set_font_map.)
How much does it change your profiles to export CPPFLAGS="-DG_DISABLE_CAST_CHECKS" when configuring Pango? GLib and GTK+ both use this flag for production builds... I need to change Pango to do the same. A quick test seems to indicate that it makes text layout overall 7-8% faster.
Created attachment 14359 [details] [review] My attempt at a fontset caching patch
The patch I've just attached: - Adds an association between PangoFcPattern and the fontset, if loaded. - Keeps the last N (currently 16) loaded fontsets refererenced. - Removes the "keep the last N freed patterns" logic, since that shouldn't be needed any more. A nice side benefit is that the number of fontsets to cache is easier to figure out than the number of fonts, since the number of fonts that need to be cached depends on the font configuration ... if you have more fonts for more different scripts, the font cache has to be larger, but not the fontset cache.
- In general I think it looks good. Putting a fontset pointer into the fcpatternset is more cleaner and more direct than adding yet another hashtable to the fontmap structure. - This fragment: + patterns->fontset = PANGO_FONTSET (simple); + g_object_add_weak_pointer (G_OBJECT (patterns->fontset), + (gpointer *)&patterns->fontset); adds a weak pointer. What happens if the fontset is refed by someone else at the time the pattern is destroyed? When the fontset is later unrefed, won't the weak pointer write NULL into a freed area then? - The cache_links in PangoFt2Font and PangoXftFont aren't used.
Right now, patterns are very seldom changed - as proof of this, look at how the table holding the patterns is created: node->fontset_hash = g_hash_table_new_full ((GHashFunc)pango_font_description_hash, (GEqualFunc)pango_font_description_equal, (GDestroyNotify)pango_font_description_free, (GDestroyNotify)pango_fc_font_set_free); It looks like at the point where it was switched from caching fontsets to caching pattern lists, the destroy function here wasn't switched along with. Though it isn't clear to me why everything on the desktop didn't just crash in RH 8 when you switched DPI or font rendering settings, since that should have called pango_xft_substitute_changed(). I'll clean that up. The cache_link fields were a left over from an earlier speedup change that was no longer needed when I removed the caching of fonts entirely. Good catch.
Actually, it's not as broken as all that... pango_fc_font_set_free() actually frees a PangoFcPatternSet.
Created attachment 14373 [details] [review] New version of patch
New version: - Renames pango_fc_font_set_free() to pango_fc_pattern_set_free() - Combines pango_fc_clear_fontset_hash_list and pango_fc_font_map_cache_clear into a single "public" entry point, since cache_clear() had to be called before pango_fc_clear_fontset_hash_list() [*] and there was no reason they needed to be called separately. - Removes the uncessary cache_link pointers [*] This reason this ordering is needed is because we don't memory manage the pattenrs in the fontset_cache queue ... we assume they are referenced by the font-description => pattern hashes.
Just for completeness: I'd be happy with any of these approaches. (== Gnumeric would be happy.)
Mon Feb 17 13:06:39 2003 Owen Taylor <otaylor@redhat.com> * pango/pangofc-fontmap.cI pango/pangoft2-fontmap.c pango/pangoxft-fontmap.c: Add caching of fontsets (#104495, initial patch and review by Soeren Sandmann) * pango/pangofc-fontmap.cI pango/pangoft2-fontmap.c pango/pangoxft-fontmap.c pango/pangoft2-private.h pango/pangoxft-private.h: Remove cache of recently freed fonts; not necessary now that we cache fontsets. * pango/pangofc-fontmap.cI (pango_fc_pattern_set_free): Rename from pango_fc_font_set_free to reflect what it actually does. * pango/pangofc-fontmap.cI pango/pangoft-fontmap.c pango/pangoxft-fontmap.c: Combine clear-the-cache functions; we didn't need separate clear-the-font-cache and clear-the-pattern-cache functions.