[webkit-reviews] review granted: [Bug 93712] CSS: Shrink RuleData by storing selector as index rather than pointer. : [Attachment 157711] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 93712: CSS: Shrink RuleData by storing selector as index rather than
pointer.
https://bugs.webkit.org/show_bug.cgi?id=93712

Attachment 157711: Patch
https://bugs.webkit.org/attachment.cgi?id=157711&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
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.