375757 - Cache gfxFont objects

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We should cache gfxFont objects in a global cache. See
http://groups.google.com/group/mozilla.dev.tech.gfx/browse_thread/thread/60ddf7dcf27388fd/817b529e1fbf6fca#817b529e1fbf6fca

My current plan is to use a hashtable and an nsExpirationTracker. The hashtable is keyed by the font family and style and contains all gfxFont objects; it does not keep a refcount on the object. The nsExpirationTracker contains font objects that have refcount zero. We modify gfxFont::Release() so that when the refcount drops to zero we don't delete the object, just put it in the nsExpirationTracker; objects are actually deleted when they expire. This way we can keep unused fonts around for a little while in the hope they'll get hit again.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Here it is. I made gfxFontStyle get copied into gfxFont because otherwise the gfxFont can outlive the style. We could do a followup where nsThebesFontMetrics doesn't need to heap-allocate its gfxFontStyle anymore. I got rid of some font caching in Windows and Pango that became redundant.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 3

[Peter Weilbacher]

Why did this patch change almost every occurrence of mStyle to GetStyle()?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Because mStyle became a member struct but a lot of uses wanted to get it as a pointer. So I was going to have to change all those uses and I thought I'd put it behind a member function in case we need to change things again later.

Comment 5

[Peter Weilbacher]

OK, yes it's more flexible. I just asked because I recently in another project I had huge wins by directly accessing variables instead of using member functions. But there these variables were accessed very often in large loops which won't be the case here, except perhaps on long pages with many different fonts.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

GetStyle() is inline so the compiler should produce the right code.