1201403 - streamline MacOSFontEntry::HasFontTable

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

Profiling textbench tests under 10.10, roughly 10% of the time spent was in MacOSFontEntry::HasFontTable. This is a query to the font as to whether a given table exists or not in the font. The implementation is copying the table data as a way to test whether a table exists. It would be better to use the available CoreGraphics call to list tables in the font and stick those into a hash table.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, streamline MacOSFontEntry::HasFontTable

Use CGFontCopyTableTags to store the set of available tables in a simple hashset.

Comment 2

[Jonathan Kew [:jfkthame]]

Interesting. Profiling on 10.9, I see less than 1% under HasFontTable. I've always understood that table data is not actually copied by the CGFontCopyTableForTag call that it uses; it just bumps the refcount of an existing CFData and returns it.

Sounds like 10.10 may have changed in such a way that this is no longer true, and CGFontCopyTableForTag has become much more expensive. :(

Comment 3

[Jonathan Kew [:jfkthame]]

Comment on attachment 8656416 [details] [diff] [review]
patch, streamline MacOSFontEntry::HasFontTable

Review of attachment 8656416 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxMacPlatformFontList.mm
@@ +380,2 @@
>  
> +       CGFontRef fontRef = GetFontRef();

nit: indentation is wrong on these two lines

@@ +381,5 @@
> +       CGFontRef fontRef = GetFontRef();
> +        if (!fontRef) {
> +            return false;
> +        }
> +        CFArrayRef tags = CGFontCopyTableTags(fontRef);

nit: for consistency with nearby code, it'd be nice to use the :: prefix on the CG call here. Same for two CF calls below.

Comment 4

[John Daggett (:jtd)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8175a26ffebf

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/219979d9d8ba

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/219979d9d8ba