Bug 80707 – Presentation attribute cache

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

[Antti Koivisto]

It is common for the same presentation attribute values repeat. We should introduce a cache that uses presentation attribute names and values as key. This will help to avoid repeated parsing of the same attribute values, reduce memory consumption and speed up the style resolve .

[Antti Koivisto]

Created attachment 131054 [details]
patch

[Early Warning System Bot]

Comment on attachment 131054 [details]
patch

Attachment 131054 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/11898650

[Early Warning System Bot]

Comment on attachment 131054 [details]
patch

Attachment 131054 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11906670

[WebKit Review Bot]

Comment on attachment 131054 [details]
patch

Attachment 131054 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11904652

[Andreas Kling]

Comment on attachment 131054 [details]
patch

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

> Source/WebCore/dom/StyledElement.cpp:73
> +    static PresentationAttributeCache* cache = new PresentationAttributeCache();
> +    return *cache;

Needs more common WebKit patterns:
DEFINE_STATIC_LOCAL(PresentationAttributeCache, cache, ());
return cache;

> Source/WebCore/dom/StyledElement.cpp:204
> +        if (!attribute->namespaceURI().isNull())
> +            return;

Perhaps this could be an assertion instead?

> Source/WebCore/dom/StyledElement.cpp:236
> +    PresentationAttributeCache::iterator cacheIterator;
> +    if (cacheHash) {
> +        cacheIterator = presentationAttributeCache().add(cacheHash, nullptr).first;

We could grab the PresentationAttributeCacheEntry* instead of holding on to a PresentationAttributeCache::iterator here to make the following code a little prettier.

[Antti Koivisto]

Created attachment 131063 [details]
try to silence the gcc warning, implement some comments

[Antti Koivisto]

(In reply to comment #5)
> We could grab the PresentationAttributeCacheEntry* instead of holding on to a PresentationAttributeCache::iterator here to make the following code a little prettier.

I need the iterator to avoid the second hash lookup in case of insertion.

[Andreas Kling]

Comment on attachment 131063 [details]
try to silence the gcc warning, implement some comments

r=me.

I wonder if we can make this cache case-insensitive. That should provide a solid increase in sharing on Y2K content.

[WebKit Review Bot]

Comment on attachment 131063 [details]
try to silence the gcc warning, implement some comments

Attachment 131063 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11894738

New failing tests:
fast/forms/input-type-change.html

[Antti Koivisto]

http://trac.webkit.org/changeset/110316 (with a fix for the failing test)