John Daggett (:jtd) Assignee
	Description • 17 years ago

This is a follow-on bug to bug 396137, the port of Windows font matching code to the Mac.  Right now, the two things I think should be improved are:

1. Cache pref font lists rather than generating them each time a character is not found in the font group font list.

As noted in comment 28 of bug 396137, profiling with the 396137 patch shows that 80% of time spent within InitTextRun for the fifty slowest pages in the talos page set is spent creating pref font lists.

2. Set up a background task of some flavor that loads in cmap information little-by-little to avoid being forced to load all cmap information at startup or on the first miss (i.e. when a character is not found either the style fonts or in the fallback pref fonts).

Suggested priority: P3

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 1 • 17 years ago

These will be cross-platform optimizations for both Windows and Mac, right?

Also, are we going to have Linux use this font matching code too? I hope the answer is yes...

	John Daggett (:jtd) Assignee
	Comment 2 • 17 years ago

(In reply to comment #1)
> These will be cross-platform optimizations for both Windows and Mac, right?

Strictly speaking, no.  The Windows code already caches the pref font lists in arrays of nsRefPtr<FontEntry> objects.  The FontEntry objects are different on Mac and Windows so this code doesn't work easily cross-platform. Right now the FontEntry class isn't particularly Windows dependent but Stuart has talked about adding more Windows-specific functionality there.  To make a cross-platform version we'd need to work out something that fits into those plans.  Masayuki has a patch to unify these across platforms but I'm not sure whether this is going in or not.

We could implement the incremental reading of cmap data cross-platform, it's currently done all at startup on Windows.
 
> Also, are we going to have Linux use this font matching code too? I hope the
> answer is yes...

The CmapFontMatcher code uses gfxAtsuiFont objects but doesn't make any ATSUI calls.  So with a little work to move some functionality into gfxFont instead we could probably figure out a way of making that cross-platform.  We also need cross-platform FontEntry objects to abstract out the methods for accessing cmaps.

As for the API's that maintain the system-wide font list under Linux, I don't really know much about that code.  You'd need to know about any sort of "font funkiness" with typical sets of fonts on Linux, which flavor of cmap's they tend to use, yadda, yadda.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 3 • 17 years ago

Oh, hmm. That makes sense.

I was thinking about cached character-to-glyph mappings for the pref fonts, but I guess that's a different bug.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 4 • 17 years ago

(In reply to comment #1)
> Also, are we going to have Linux use this font matching code too? I hope the
> answer is yes...

I think the current answer is 'no', based on discussions with behdad; however, if font selection ends up being a big bottleneck on linux we may need to.  The reasoning is that to be fully linux-compliant or whatever we need to use fontconfig for font selection, so that it can apply the user's fontconfig preferences and any custom configuration/matching bits.

That said, if the port wouldn't be too hard, it would be useful to benchmark this code on linux, but that's a separate bug too.

	Mike Schroepfer
	Comment 5 • 17 years ago

+ing since this can make a major difference to Tp

	John Daggett (:jtd) Assignee
	Comment 6 • 17 years ago

I'm adding a dependency on 404310, because to be able to cache pref fonts in an efficient manner I need to make changes to the structure of the hashtables in gfxQuartzFontCache.  These will change with the work that's required for 404310.

	John Daggett (:jtd) Assignee
	Comment 7 • 17 years ago

Instead of building the pref font lists for *each* character, cache the list of font families per lang group (this never changes unless the pref settings are changed), and make gfxAtsuiFont objects only when the font contains a codepoint in its cmap.  Also, cache away CJK preferences based on accept-language and system default script.

	Mike Schroepfer
	Comment 8 • 17 years ago

We want this for b3?

	John Daggett (:jtd) Assignee
	Comment 9 • 17 years ago

Comment on attachment 300294 [details] [diff] [review]
patch, v.0.1, cache pref font lists more efficiently

This is a Mac-only fix, it should give us a good improvement in Tp loadtime performance.

	Stuart Parmenter
	Comment 10 • 17 years ago

we should take this, imho.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 11 • 17 years ago

Comment on attachment 300294 [details] [diff] [review]
patch, v.0.1, cache pref font lists more efficiently

a=beltzner for beta 3

	philippe (part-time)
	Comment 12 • 17 years ago

With this patch applied I get poor display of Japanese characters on pages with utf-8 encoding (google) or/and stylesheets that specify only roman fonts, like
http://mozillazine.jp/
http://www.google.com/search?q=mozilla%20japan&ie=utf-8&oe=utf-8

	philippe (part-time)
	Comment 13 • 17 years ago

screenshot for comment 12
both 10.4 and 10.5
Camino build,  2.0a1pre (1.9b3pre 2008013017)

OS is English

	philippe (part-time)
	Comment 14 • 17 years ago

That is the same issue that was originally fixed in bug 35755.

	philippe (part-time)
	Comment 15 • 17 years ago

er, I meant bug 357555.
sorry for the bugspam.

	John Daggett (:jtd) Assignee
	Comment 16 • 17 years ago

Argh, my bad.  Include eFontPrefLang_CJKSet as a CJK lang and handle pref lang array correctly.

	Mike Schroepfer
	Comment 17 • 17 years ago

Comment on attachment 300572 [details] [diff] [review]
patch, fixup screwup handling CJK lang prefs

assuming this passes review let's take this

	philippe (part-time)
	Comment 18 • 17 years ago

(In reply to comment #16)
> Created an attachment (id=300572) [details]
yup, that fixes the issue on 10.5.1 on my local build

	John Daggett (:jtd) Assignee
	Comment 19 • 17 years ago

For pages that hit the pref fonts, this adds a nice speedup since in most cases the last pref font hit will be the same for other characters in the same unicode range.  The best case fast path can only be used by the first pref font in the list, in other cases need to check through the pref font list in order.

	John Daggett (:jtd) Assignee
	Comment 20 • 17 years ago

Comment on attachment 300773 [details] [diff] [review]
patch, cache the last pref font looked up

This is a simple speedup tweak with no change in behavior.  It will only affect pages that often fall through to pref fonts, like Chinese pages that specify Arial for everything.

	John Daggett (:jtd) Assignee
	Comment 21 • 17 years ago

Uses real-time clock to count cycles for font matching process within InitTextRun.  Dumps the text run with style and font matching info, along with cycle counts per character and the stage at which the a font was matched within gfxAtsuiFontGroup::FindFontForChar.  The code appended to the count means:

jn joiner
fg font group
pu private use char
pr pref font
pv prev font
sy system font lookup
nm not matched

Handy for figuring out what fonts are matching to particular text runs and why.

	Stuart Parmenter
	Comment 22 • 17 years ago

Comment on attachment 300773 [details] [diff] [review]
patch, cache the last pref font looked up

please expand the comment: // check to see if this is the last pref family looked up

just to be a little more specific on what is going on and in what conditions we are likely to hit that case.

	John Daggett (:jtd) Assignee
	Comment 23 • 17 years ago

created bug 416923 to deal with moving gfxSparseBitSet over to xpcom

	John Daggett (:jtd) Assignee
	Comment 24 • 17 years ago

additional comment description added:

// if a pref font is used, it's likely to be used again in the same text run.
// the style doesn't change so the face lookup can be cached rather than calling
// GetOrMakeFont repeatedly.  speeds up FindFontForChar lookup times for subsequent
// pref font lookups

	John Daggett (:jtd) Assignee
	Comment 25 • 17 years ago

patch for caching last pref font checked in.

With this the tasks for item 1 in the description are pretty much wrapped up (always room for performance improvements, naturally!).  Font cmaps are still loaded lazily, this was task 2, so the first page to cause a system-wide font search will take a performance hit.  Several patches have already been checked in for this bug, so I'm going to move this task out to a separate bug, bug 416946.