[Webkit-unassigned] [Bug 76337] Cache CSSStyleSelector::Features in RuleSets
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jan 14 12:26:30 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=76337
--- Comment #2 from Andreas Kling <kling at webkit.org> 2012-01-14 12:26:28 PST ---
(From update of attachment 122554)
View in context: https://bugs.webkit.org/attachment.cgi?id=122554&action=review
Dumping superficial comments since you have to reup anyway.
> Source/WebCore/ChangeLog:13
> + This is 1-2% CPU time reduction (engadged, nytimes) due less time spent in feature collection.
Typo, engadget.
> Source/WebCore/ChangeLog:25
> + Add a field far caching the features.
Typo, for.
> Source/WebCore/css/CSSStyleSelector.cpp:225
> + void addRule(CSSStyleRule* , CSSSelector*);
Extra space after CSSStyleRule*.
> Source/WebCore/css/CSSStyleSelector.cpp:230
> +
Extra whitespace.
> Source/WebCore/css/CSSStyleSelector.cpp:1950
> + if (selector->m_match == CSSSelector::Id && !selector->value().isEmpty())
I suspect selector->value() can't be empty if selector->m_match == CSSSelector::Id.
No need to change that in this patch as you're just moving the code though.
> Source/WebCore/css/CSSStyleSelector.cpp:1980
> + if (selector->isSiblingSelector())
> + foundSiblingSelector = true;
You could change this to:
if (!foundSiblingSelector && selector->isSiblingSelector())
To avoid wasting cycles after a first sibling selector is found.
> Source/WebCore/css/CSSStyleSelector.h:225
> + struct RuleSelectorPair {
> + RuleSelectorPair(CSSStyleRule* rule, CSSSelector* selector) : rule(rule), selector(selector) { }
Seems like we should either hide RuleSelectorPair() or make it initialize rule and selector as null pointers.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list