Bug 79195 - Cache <font face> family lists in CSSValuePool.
Summary: Cache <font face> family lists in CSSValuePool.
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
Depends on:
Blocks: 78070
  Show dependency treegraph
Reported: 2012-02-21 22:14 PST by Andreas Kling
Modified: 2012-02-21 23:33 PST (History)
0 users

See Also:

Semidecent patch (6.26 KB, patch)
2012-02-21 22:29 PST, Andreas Kling
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2012-02-21 22:14:48 PST
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.
Comment 1 Andreas Kling 2012-02-21 22:29:02 PST
Created attachment 128130 [details]
Semidecent patch
Comment 2 Antti Koivisto 2012-02-21 22:41:15 PST
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
Comment 3 Andreas Kling 2012-02-21 23:33:36 PST
Committed r108451: <http://trac.webkit.org/changeset/108451>