[webkit-reviews] review granted: [Bug 74269] Implement a cache for CSSStyleRule::selectorText() : [Attachment 118782] [PATCH] ChangeLog entry augmented

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has granted Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 74269: Implement a cache for CSSStyleRule::selectorText()
https://bugs.webkit.org/show_bug.cgi?id=74269

Attachment 118782: [PATCH] ChangeLog entry augmented
https://bugs.webkit.org/attachment.cgi?id=118782&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118782&action=review


r=me, but please inline cleanup() before landing.

> Source/WebCore/css/CSSStyleRule.cpp:51
> +    cleanup();

I'd rather we inline cleanup() here to avoid introducing a perf regression
(however tiny) in document teardown.

> Source/WebCore/css/CSSStyleRule.cpp:126
> +    if (m_hasCachedSelectorText) {
> +	   ASSERT(selectorTextCache().contains(this));
> +	   selectorTextCache().remove(this);
> +	   m_hasCachedSelectorText = false;
> +    }
> +
>      if (this->selectorText() == oldSelectorText)
>	   return;

We could factor this differently to reduce hash map churn (the remove() is
always followed by a set().)