1628419 - macOS platform back-end holds onto a CGFontRef for every installed font

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

In looking into bug 1625476, I notice that the macOS font backend will normally create a CGFontRef for each font on the platform (because it uses this to read the font metadata we need for font selection etc), and this font object is then held alive for the entire process lifetime.

Given that there will typically be hundreds or even thousands of individual font faces, of which only a few will actually be used for text rendering, this seems like it may be putting unnecessary pressure on CoreGraphics resources. I think we should avoid holding on to a reference for every font as a result of the initial scan of available fonts, and instead only do this for fonts that are actually used.

(Unfortunately, fixing this does not appear to actually resolve bug 1625476; it looks like once the font has been touched, CG is keeping the file open even after we drop our reference to it. Still, we should do our part by not holding on to resources we don't need, so that the OS at least has the option to flush things that are no longer referenced, if it wishes.)

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1628419 - Don't keep a live CGFont reference in every macOS font entry; only retain it for fonts actually used. r=lsalzman

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e346ea9f285
Don't keep a live CGFont reference in every macOS font entry; only retain it for fonts actually used. r=lsalzman

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jonathan, could this have an impact (full or even partly) for bug 1625590?

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #3)

Jonathan, could this have an impact (full or even partly) for bug 1625590?

I don't think so -- IIUC that's about actual font data that's being (unnecessarily?) serialized and sent over IPC, which is a separate issue from the font list retaining references to CoreGraphics font instances. (But feel free to re-check with a build that includes this patch, in case I'm misunderstanding something!)

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sure, once it has been merged to central I will create a new DMD build for MacOS and keep an eye on it over the next days. Thanks.

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2e346ea9f285