[webkit-reviews] review canceled: [Bug 61875] Fonts returned by FontCache::getFontDataForCharacters() are never released : [Attachment 95641] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 1 15:50:56 PDT 2011


Michael Saboff <msaboff at apple.com> has canceled Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 61875: Fonts returned by FontCache::getFontDataForCharacters() are never
released
https://bugs.webkit.org/show_bug.cgi?id=61875

Attachment 95641: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=95641&action=review

------- Additional Comments from Michael Saboff <msaboff at apple.com>
Made changes to address most of the comments.

Added speculative changes to FontCache::getFontDataForCharacters for other
platforms.

I did reduce the number of hash lookups in the FontCache::assureOneHold case by
keeping the top of m_autoReleaseStack value.  

Concerning the comments and questions about moving to a reference counting
algorithm. That is my desire as well, but I have some lingering concerns due to
performance.  I instrumented the current code to compare the calls to
FontCache::assureOneHold, which only happen for system fallback fonts, with the
likely number of ref() calls in a reference counting implementation.  On one
memory tests, the ratio was ~13 likely ref() calls to 1 assureOneHold call. 
For english only web pages, there were 0 or very few FontCache::assureOneHold
calls and thousands to tens of thousands of likely ref() calls.

I will be modifying this patch to replace the reference counting currently
implemented in the FontCache class with RefPtr<FontData> for the holders of
font data down stream of the glyph page node tree.  I will then compare this
patch's performance to the reference counting version.	If the reference
counting version performs the same or better than this patch, I will post it
for review.


More information about the webkit-reviews mailing list