Skip to content

Fix font.glyph lifetimes allow proxy glyphs in cache

Jeremy Soller requested to merge alexheretic:glyph-lifetime into master

Created by: alexheretic

One thing I noticed about Cache::queue_glyph was that it called glyph.standalone() for each queued glyph to convert it to a 'static lifetime. I wondered why this was necessary, and when it wasn't if the performance would be improved.

Allowing the cache's queue to take glyphs with a generic lifetime is fairly simple, but I had to stare at the error messages for a while to work out why it wasn't working. There was a signature error in Font::glyph:

Before

impl<'a> Font<'a> {
    pub fn glyph<C: Into<CodepointOrGlyphId>>(&self, id: C) -> Option<Glyph> {...}
}

After

impl<'a> Font<'a> {
    pub fn glyph<C: Into<CodepointOrGlyphId>>(&self, id: C) -> Option<Glyph<'a>> {...}
}

Spot the difference? Before the signature means the Glyph takes the lifetime of the reference to the font, rather than the lifetime of the font data. This was compounded by the fact a reference to the font was used in the GlyphInner struct. This isn't necessary, as Font is a cheap-clone type anyway (a reference, or an Arc).

With this change we can see another small improvement to cache performance, in addition to the optimisation earlier today:

name                                                    control.stdout ns/iter  change.stdout ns/iter  diff ns/iter  diff %  speedup 
gpu_cache::cache_bench_tests::cache_bench_tolerance_1   3,048,049               2,878,432                  -169,617  -5.56%   x 1.06 
gpu_cache::cache_bench_tests::cache_bench_tolerance_p1  5,288,473               5,067,874                  -220,599  -4.17%   x 1.04

Moreover, this has a larger affect downstream. In my crate gfx_glyph I'm seeing 15-24% reduction in cache interaction latency after all standalone() calls are removed (allowed by this patch).

The change from Cache -> Cache<'font> means this isn't totally backward compatible though, so should be part of the next version, ie 0.3.

Merge request reports