Bug 93712 – CSS: Shrink RuleData by storing selector as index rather than pointer.

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

[Andreas Kling]

CSS: Shrink RuleData by storing selector as index rather than pointer.

[Andreas Kling]

Created attachment 157711 [details]
Patch

[Antti Koivisto]

Comment on attachment 157711 [details]
Patch

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

r=me

Better keep eye on performance bots after landing.

> Source/WebCore/css/CSSSelectorList.h:53
> +    size_t indexAfter(size_t index) const
> +    {

This could use a more descriptive name. No great suggestions though.

> Source/WebCore/css/StyleResolver.cpp:229
> +    unsigned m_selectorIndex : 12;
>      // This number was picked fairly arbitrarily. We can probably lower it if we need to.
>      // Some simple testing showed <100,000 RuleData's on large sites.
> -    unsigned m_position : 24;
> +    unsigned m_position : 20;

So we now support 4k selectors per rule and 1M active style rules per document. That sounds more than sufficient but we probably shouldn't shave many more bit here.

If the limits are overrun we get semi-random matching behavior (wrong selectors gets tested, rules applied in wrong order,...). I wonder we should add overflow testing and simply ignore overflowing rules/selectors.

[Antti Koivisto]

Comment on attachment 157711 [details]
Patch

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

> Source/WebCore/css/StyleResolver.cpp:230
> +    unsigned m_specificity : 24;

Specificity shouldn't really need nearly this many bits. The computation is just not implemented very space-efficiently.

[Andreas Kling]

Created attachment 157723 [details]
Latch for panding

[WebKit Review Bot]

Comment on attachment 157723 [details]
Latch for panding

Clearing flags on attachment: 157723

Committed r125294: <http://trac.webkit.org/changeset/125294>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Florin Malita]

Looks like this broke the Android & Qt builds:

Source/WebCore/css/StyleResolver.cpp:248: error: size of array 'RuleData_should_stay_small' is negative

[Florin Malita]

I've committed http://trac.webkit.org/changeset/125305 to get the builds going, but that's probably wasting sizeof(unsigned) on 32-bit platforms due to uint64 alignment.

I'll post a patch shortly to address that.