search

googlefonts / fontc

Where in we pursue oxidizing (context: https://github.com/googlefonts/oxidize) fontmake.
Apache License 2.0
62 stars 10 forks source link

[g2f] Use smolstr when parsing glyphs.app data #765

Closed cmyr closed 3 months ago

cmyr commented 3 months ago

Particularly for types that end up being SmolStr later on, like glyph names; it's inconvenient and inefficient to parse to String and then later convert to SmolStr, so this is a minor simplification.

Arguably the whole plist parsing process should be using smolstr, which should offer a reasonable speedup, but we can worry about that when we're thinking more generally about optimization.

...looking at this again, maybe it would make sense to use GlyphName whenever we're dealing with a glyph name? that's the type we're going to want later anyway, and it would save us from needing to convert things later...

rsheeter commented 3 months ago

Is the impact detectable (https://github.com/googlefonts/fontc?tab=readme-ov-file#comparing-branch-performance-with-hyperfine)? - happy to land either regardless, just curious.

cmyr commented 3 months ago

no it's not... if there's any benefit I think you'd need to have a parsing-specific benchmark for it to show up. Mostly about ergonomics in any case. :)