Bug 79195 – Cache <font face> family lists in CSSValuePool.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

Legacy content is typically quite heavy on <font face>. Parsing the face attribute to create a list of families is very expensive. We should cache the results of parsing a given attribute value to minimize time spent here.

[Andreas Kling]

Created attachment 128130 [details]
Semidecent patch

[Antti Koivisto]

Comment on attachment 128130 [details]
Semidecent patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128130&action=review

r=me with a mechanism to prevent unlimited pool growth in weird cases.

> Source/WebCore/ChangeLog:10
> +        HTMLFontElements with "face" attributes are very common in legacy web content.
> +        Add a String->CSSValue cache for these in CSSValuePool and use it to avoid
> +        reparsing and recreating duplicate font face values.

A word about performance results might be good.

> Source/WebCore/css/CSSValuePool.cpp:140
> +PassRefPtr<CSSValueList> CSSValuePool::createFontFaceValue(const AtomicString& string, CSSStyleSheet* contextStyleSheet)
> +{
> +    RefPtr<CSSValueList>& value = m_fontFaceValueCache.add(string, 0).first->second;
> +    if (!value)
> +        value = CSSParser::parseFontFaceValue(string, contextStyleSheet);
> +    return value;
> +}

There should be a mechanism to prevent boundless growth in case this gets hits repeatedly with different values. Something similar to what color value pool has should work fine.

> Source/WebCore/css/CSSValuePool.h:81
>      IntegerValueCache m_percentValueCache;
>      IntegerValueCache m_numberValueCache;
>  
> +    typedef HashMap<AtomicString, RefPtr<CSSValueList> > FontFaceValueCache;
> +    FontFaceValueCache m_fontFaceValueCache;

It would be nice to renames all these Caches to Pools at some point

[Andreas Kling]

Committed r108451: <http://trac.webkit.org/changeset/108451>