Changeset 149557 in webkit

Timestamp:

May 4, 2013 8:03:26 AM (11 years ago)

Author:

akling@apple.com

Message:

Remove CSS selector profiler branches from ElementRuleCollector loop.
<​http://webkit.org/b/115581>

Reviewed by Antti Koivisto.

Templatize the method so we don't have to check for active inspector frontends on every
pass through the loop.

Time spent in this loop goes down from 0.5% to 0.2% on iTunes Store, WebCore binary size
goes up 480 bytes (sorry Benjamin, I'll make it up to you.)

• css/ElementRuleCollector.h:
• css/ElementRuleCollector.cpp:

(WebCore::ElementRuleCollector::collectMatchingRulesForList):
(WebCore::ElementRuleCollector::doCollectMatchingRulesForList):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/ElementRuleCollector.cpp (modified) (5 diffs)
• css/ElementRuleCollector.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-05-03  Andreas Kling  <akling@apple.com>
+
+        Remove CSS selector profiler branches from ElementRuleCollector loop.
+        <http://webkit.org/b/115581>
+
+        Reviewed by Antti Koivisto.
+
+        Templatize the method so we don't have to check for active inspector frontends on every
+        pass through the loop.
+
+        Time spent in this loop goes down from 0.5% to 0.2% on iTunes Store, WebCore binary size
+        goes up 480 bytes (sorry Benjamin, I'll make it up to you.)
+
+        * css/ElementRuleCollector.h:
+        * css/ElementRuleCollector.cpp:
+        (WebCore::ElementRuleCollector::collectMatchingRulesForList):
+        (WebCore::ElementRuleCollector::doCollectMatchingRulesForList):
+
 2013-04-30  Robert Hogan  <robert@webkit.org>
 

trunk/Source/WebCore/css/ElementRuleCollector.cpp

@@ -404 +404 @@
 void ElementRuleCollector::collectMatchingRulesForList(const Vector<RuleData>* rules, const MatchRequest& matchRequest, StyleResolver::RuleRange& ruleRange)
 {
+    if (UNLIKELY(InspectorInstrumentation::hasFrontends())) {
+        doCollectMatchingRulesForList<true>(rules, matchRequest, ruleRange);
+        return;
+    }
+    doCollectMatchingRulesForList<false>(rules, matchRequest, ruleRange);
+}
+
+template<bool hasInspectorFrontends>
+void ElementRuleCollector::doCollectMatchingRulesForList(const Vector<RuleData>* rules, const MatchRequest& matchRequest, StyleResolver::RuleRange& ruleRange)
+{
     if (!rules)
         return;
@@ -416 +426 @@
 
         StyleRule* rule = ruleData.rule();
-        InspectorInstrumentationCookie cookie = InspectorInstrumentation::willMatchRule(document(), rule, m_inspectorCSSOMWrappers, document()->styleSheetCollection());
+        InspectorInstrumentationCookie cookie;
+        if (hasInspectorFrontends)
+            cookie = InspectorInstrumentation::willMatchRule(document(), rule, m_inspectorCSSOMWrappers, document()->styleSheetCollection());
         PseudoId dynamicPseudo = NOPSEUDO;
         if (ruleMatches(ruleData, matchRequest.scope, dynamicPseudo)) {
@@ -422 +434 @@
             const StylePropertySet* properties = rule->properties();
             if (!properties || (properties->isEmpty() && !matchRequest.includeEmptyRules)) {
-                InspectorInstrumentation::didMatchRule(cookie, false);
+                if (hasInspectorFrontends)
+                    InspectorInstrumentation::didMatchRule(cookie, false);
                 continue;
             }
             // FIXME: Exposing the non-standard getMatchedCSSRules API to web is the only reason this is needed.
             if (m_sameOriginOnly && !ruleData.hasDocumentSecurityOrigin()) {
-                InspectorInstrumentation::didMatchRule(cookie, false);
+                if (hasInspectorFrontends)
+                    InspectorInstrumentation::didMatchRule(cookie, false);
                 continue;
             }
@@ -434 +448 @@
             if (dynamicPseudo != NOPSEUDO && m_pseudoStyleRequest.pseudoId == NOPSEUDO) {
                 if (m_mode == SelectorChecker::CollectingRules) {
-                    InspectorInstrumentation::didMatchRule(cookie, false);
+                    if (hasInspectorFrontends)
+                        InspectorInstrumentation::didMatchRule(cookie, false);
                     continue;
                 }
@@ -447 +462 @@
                 // Add this rule to our list of matched rules.
                 addMatchedRule(&ruleData);
-                InspectorInstrumentation::didMatchRule(cookie, true);
+                if (hasInspectorFrontends)
+                    InspectorInstrumentation::didMatchRule(cookie, true);
                 continue;
             }
         }
-        InspectorInstrumentation::didMatchRule(cookie, false);
+        if (hasInspectorFrontends)
+            InspectorInstrumentation::didMatchRule(cookie, false);
     }
 }

trunk/Source/WebCore/css/ElementRuleCollector.h

@@ -93 +93 @@
         
 private:
+    template<bool hasInspectorFrontends>
+    void doCollectMatchingRulesForList(const Vector<RuleData>*, const MatchRequest&, StyleResolver::RuleRange&);
+
     const StyleResolver::State& m_state;
     DocumentRuleSets& m_ruleSets;