Bug 14601 - WebCore/Windows: too many FontData object and takes too much memory
Summary: WebCore/Windows: too many FontData object and takes too much memory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Windows XP
: P1 Major
Assignee: Dave Hyatt
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2007-07-13 03:35 PDT by 808caaa4.8ce9.9cd6c799e9f6
Modified: 2007-07-19 14:21 PDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description 808caaa4.8ce9.9cd6c799e9f6 2007-07-13 03:35:27 PDT
In WebCore::FontCache::getFontDataForCharacters(),
many many FontData/FontPlatformData objects created with same constructor parameters.

HFONT by IMLangFontLink2 is cached result(documented in MSDN docs).
FontPlatformData objects have CFFont object, work buffer, for each.

FontData/FontPlatformData objects live as long as the process lives.
Another saying, objects isn't destroyed even after closing assosiated views.


Example: viewing news.google.co.jp, with caching FontData objects like this:

-------------------------
.../WebCore/platform/win/FontCacheWin.cpp
-------------------------
fontData = new FontData(FontPlatformData(result, font.fontDescription().computedPixelSize(), font.fontDescription().bold(), font.fontDescription().italic()));
->
fontData = CreateFontDataWithCache(result, font.fontDescription().computedPixelSize(), font.fontDescription().bold(), font.fontDescription().italic());
-------------------------

Results: only 8 objects created, and almost 1400 objects avoided from creation,
30MB memory seems to be saved (71MB used without cache, 40MB used with cache.)
Comment 1 David Kilzer (:ddkilzer) 2007-07-14 15:19:29 PDT
<rdar://problem/5335829>
Comment 2 Dave Hyatt 2007-07-16 13:08:38 PDT
I think this should be P1.
Comment 3 Dave Hyatt 2007-07-16 13:15:17 PDT
Actually lowering this back to P2.  This case should only get hit if no font specified in CSS contains the right characters.  I'm not completely sure how we would go about caching this result, since the value returned is dependent on the characters being used.
Comment 4 Dave Hyatt 2007-07-16 13:16:48 PDT
Oh, never mind, I get it.
Comment 5 Dave Hyatt 2007-07-16 13:22:19 PDT
This is definitely a P1 issue.  Mac is actually using the cache for this method, but Windows is creating a new FontData every time.
Comment 6 Dave Hyatt 2007-07-16 13:48:43 PDT
My assumption was that mlang would give me different HFONTs, but if it really does cache fonts internally, then we can simply use our cache on top of mlang.
Comment 7 808caaa4.8ce9.9cd6c799e9f6 2007-07-17 02:38:08 PDT
errata: not CFFont, is CGFont, of course.

http://msdn2.microsoft.com/En-US/library/aa741043.aspx
> IMLangFontLink does (font linking) by creating custom fonts and 
> providing an underlying font cache in the implementation.

Actually same (DWORD)HFONT returns many times, and
number of GDI objects NOT SO increased.
Comment 8 David Kilzer (:ddkilzer) 2007-07-17 07:10:58 PDT
Fixed in r24332.
Comment 9 808caaa4.8ce9.9cd6c799e9f6 2007-07-18 18:32:49 PDT
new impl: beautiful.

-----
FontPlatformData platformData(result, ...);
fontData = getCachedFontData(&platformData);
fontData->setIsMLangFont();
-----

I fear if this temporary platformData's associated objects is successfully deleted....
(Of cource, platformData itself is on local stack.)

-----
FontPlatformData::~FontPlatformData()
{
}
-----

In FE environment this constructor is called for almost every every every FE chars.
Constructor checks and fills for m_synthetic***s with calling 
GetOutlineTextMetricsW(),EnumFontFamiliesEx() each time, and creates CGFont inside.

Checking before creating FontPlatformData objects is with reality, 
rather than implementing destructors or delay-CGFont-creation, I thought.


Additional info. (might be another issue but related)
IMLangFontLink certainly caches our HFONTs, but it's cache slot might be too small?
langFontLink->MapFont() seems returns E_INVALID so often with correct arguments... why?
I'm working now for verification about this hypnosis.
If proofed, I'll report here and ask for instructions.
Comment 10 Dave Hyatt 2007-07-19 14:21:20 PDT
There is an underlying CGFontRef cache, so we are not creating new CGFontRefs every time.