Closed Bug 375757 Opened 18 years ago Closed 18 years ago

Cache gfxFont objects

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file)

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.
Attached patch fix (deleted) — Splinter Review
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.
Attachment #260309 - Flags: review?(pavlov)
Attachment #260309 - Flags: review?(pavlov) → review+
checked in.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Why did this patch change almost every occurrence of mStyle to GetStyle()?
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.
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.
GetStyle() is inline so the compiler should produce the right code.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: