Optimize stylesheet insertions
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2012 14:52:19 +0000 (14:52 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Sep 2012 14:52:19 +0000 (14:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97627

Reviewed by Andreas Kling.

PerformanceTests:

Add synthetic performance test for avoiding style recalcs on stylesheet inserts.

* CSS/StyleSheetInsert.html: Added.

Source/WebCore:

We currently do scope analysis for stylesheets that are added to the end of the active stylesheet list to avoid unnecessary style
recalcs and StyleResolver rebuilding. However it is somewhat common to insert <style> elements dynamically to positions other than last.
In this case we currently simply force full style recalc. We should do scope analysis and partial style recalcs also in these cases.

PerformanceTests/CSS/StyleSheetInsert.html microbenchmark shows ~20x progression from the patch.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::StyleResolver):
(WebCore::StyleResolver::resetAuthorStyle):

    Add a way to reset author RuleSet without deleting the whole StyleResolver.

(WebCore):
* css/StyleResolver.h:
(StyleResolver):
* dom/DocumentStyleSheetCollection.cpp:
(WebCore::DocumentStyleSheetCollection::analyzeStyleSheetChange):

    Check if there have been insertions to the stylesheet list. If so we need to reset
    the StyleResolver (to handle rule position changes) but don't need to force full
    style recalc. Do scope analysis for inserted stylesheets as well.

(WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets):
* dom/DocumentStyleSheetCollection.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@129644 268f45cc-cd09-0410-ab3c-d52691b4dbfc

PerformanceTests/CSS/StyleSheetInsert.html [new file with mode: 0644]
PerformanceTests/ChangeLog
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h
Source/WebCore/dom/DocumentStyleSheetCollection.cpp
Source/WebCore/dom/DocumentStyleSheetCollection.h

diff --git a/PerformanceTests/CSS/StyleSheetInsert.html b/PerformanceTests/CSS/StyleSheetInsert.html
new file mode 100644 (file)
index 0000000..a1fa082
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../resources/runner.js"></script>
+</head>
+<body>
+<iframe></iframe>
+</body>
+<script>
+var frame = document.getElementsByTagName("iframe")[0];
+var testDoc = frame.contentDocument;
+var docText = "";
+docText += "<body><style>.foo {color:red}</style>";
+docText += "<div class='bar'>Foo</div>";
+for (var i = 0; i < 10000; ++i)
+    docText += "<div class='foo'>Foo</div>";
+testDoc.body.innerHTML = docText;
+
+PerfTestRunner.run(function() {
+    var styleElem = testDoc.createElement("style");
+    styleElem.innerText = ".bar {color:green}";
+    testDoc.body.insertBefore(styleElem, testDoc.body.firstChild);
+}, 50, 10);
+</script>
+</html>
index 5929dc8..10f6196 100644 (file)
@@ -1,3 +1,14 @@
+2012-09-25  Antti Koivisto  <antti@apple.com>
+
+        Optimize stylesheet insertions
+        https://bugs.webkit.org/show_bug.cgi?id=97627
+
+        Reviewed by Andreas Kling.
+
+        Add synthetic performance test for avoiding style recalcs on stylesheet inserts.
+
+        * CSS/StyleSheetInsert.html: Added.
+
 2012-09-25  Ryosuke Niwa  <rniwa@webkit.org>
 
         Skip Dromaeo/jslib-modify-jquery.html per bug 95376.
index fc2cb92..7753783 100644 (file)
@@ -1,3 +1,35 @@
+2012-09-25  Antti Koivisto  <antti@apple.com>
+
+        Optimize stylesheet insertions
+        https://bugs.webkit.org/show_bug.cgi?id=97627
+
+        Reviewed by Andreas Kling.
+
+        We currently do scope analysis for stylesheets that are added to the end of the active stylesheet list to avoid unnecessary style
+        recalcs and StyleResolver rebuilding. However it is somewhat common to insert <style> elements dynamically to positions other than last.
+        In this case we currently simply force full style recalc. We should do scope analysis and partial style recalcs also in these cases.
+        
+        PerformanceTests/CSS/StyleSheetInsert.html microbenchmark shows ~20x progression from the patch.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::StyleResolver):
+        (WebCore::StyleResolver::resetAuthorStyle):
+        
+            Add a way to reset author RuleSet without deleting the whole StyleResolver.
+
+        (WebCore):
+        * css/StyleResolver.h:
+        (StyleResolver):
+        * dom/DocumentStyleSheetCollection.cpp:
+        (WebCore::DocumentStyleSheetCollection::analyzeStyleSheetChange):
+        
+            Check if there have been insertions to the stylesheet list. If so we need to reset
+            the StyleResolver (to handle rule position changes) but don't need to force full
+            style recalc. Do scope analysis for inserted stylesheets as well.
+
+        (WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets):
+        * dom/DocumentStyleSheetCollection.h:
+
 2012-09-26  Allan Sandfeld Jensen  <allan.jensen@digia.com>
 
         Gesture tap highlighting entire first line
index 1b47c58..5680c43 100644 (file)
@@ -425,11 +425,7 @@ StyleResolver::StyleResolver(Document* document, bool matchAuthorAndUserStyles)
     if (m_rootDefaultStyle && view)
         m_medium = adoptPtr(new MediaQueryEvaluator(view->mediaType(), view->frame(), m_rootDefaultStyle.get()));
 
-    m_authorStyle = RuleSet::create();
-    // Adding rules from multiple sheets, shrink at the end.
-    // Adding global rules from multiple sheets, shrink at the end.
-    // Note that there usually is only 1 sheet for scoped rules, so auto-shrink-to-fit is fine.
-    m_authorStyle->disableAutoShrinkToFit();
+    resetAuthorStyle();
 
     DocumentStyleSheetCollection* styleSheetCollection = document->styleSheetCollection();
     // FIXME: This sucks! The user sheet is reparsed every time!
@@ -547,6 +543,12 @@ inline RuleSet* StyleResolver::ruleSetForScope(const ContainerNode* scope) const
 }
 #endif
 
+void StyleResolver::resetAuthorStyle()
+{
+    m_authorStyle = RuleSet::create();
+    m_authorStyle->disableAutoShrinkToFit();
+}
+
 void StyleResolver::appendAuthorStylesheets(unsigned firstNew, const Vector<RefPtr<StyleSheet> >& stylesheets)
 {
     // This handles sheets added to the end of the stylesheet list only. In other cases the style resolver
index 2e530f0..cadcb43 100644 (file)
@@ -159,7 +159,8 @@ public:
     void setEffectiveZoom(float f) { m_fontDirty |= style()->setEffectiveZoom(f); }
     void setTextSizeAdjust(bool b) { m_fontDirty |= style()->setTextSizeAdjust(b); }
     bool hasParentNode() const { return m_parentNode; }
-    
+
+    void resetAuthorStyle();
     void appendAuthorStylesheets(unsigned firstNew, const Vector<RefPtr<StyleSheet> >&);
     
     // Find the ids or classes the selectors on a stylesheet are scoped to. The selectors only apply to elements in subtrees where the root element matches the scope.
index 70f5427..c737792 100644 (file)
@@ -374,9 +374,9 @@ bool DocumentStyleSheetCollection::testAddedStyleSheetRequiresStyleRecalc(StyleS
     return false;
 }
 
-void DocumentStyleSheetCollection::analyzeStyleSheetChange(UpdateFlag updateFlag, const Vector<RefPtr<StyleSheet> >& newStylesheets, bool& requiresStyleResolverReset, bool& requiresFullStyleRecalc)
+void DocumentStyleSheetCollection::analyzeStyleSheetChange(UpdateFlag updateFlag, const Vector<RefPtr<StyleSheet> >& newStylesheets, StyleResolverUpdateType& styleResolverUpdateType, bool& requiresFullStyleRecalc)
 {
-    requiresStyleResolverReset = true;
+    styleResolverUpdateType = Reconstruct;
     requiresFullStyleRecalc = true;
     
     // Stylesheets of <style> elements that @import stylesheets are active but loading. We need to trigger a full recalc when such loads are done.
@@ -397,25 +397,41 @@ void DocumentStyleSheetCollection::analyzeStyleSheetChange(UpdateFlag updateFlag
     if (!m_document->styleResolverIfExists())
         return;
 
-    // See if we are just adding stylesheets.
+    // Find out which stylesheets are new.
     unsigned oldStylesheetCount = m_authorStyleSheets.size();
     if (newStylesheetCount < oldStylesheetCount)
         return;
-    for (unsigned i = 0; i < oldStylesheetCount; ++i) {
-        if (m_authorStyleSheets[i] != newStylesheets[i])
+    Vector<StyleSheet*> addedSheets;
+    unsigned newIndex = 0;
+    for (unsigned oldIndex = 0; oldIndex < oldStylesheetCount; ++oldIndex) {
+        if (newIndex >= newStylesheetCount)
             return;
+        while (m_authorStyleSheets[oldIndex] != newStylesheets[newIndex]) {
+            addedSheets.append(newStylesheets[newIndex].get());
+            ++newIndex;
+            if (newIndex == newStylesheetCount)
+                return;
+        }
+        ++newIndex;
     }
-    requiresStyleResolverReset = false;
+    bool hasInsertions = !addedSheets.isEmpty();
+    while (newIndex < newStylesheetCount) {
+        addedSheets.append(newStylesheets[newIndex].get());
+        ++newIndex;
+    }
+    // If all new sheets were added at the end of the list we can just add them to existing StyleResolver.
+    // If there were insertions we need to re-add all the stylesheets so rules are ordered correctly.
+    styleResolverUpdateType = hasInsertions ? Reset : Additive;
 
     // If we are already parsing the body and so may have significant amount of elements, put some effort into trying to avoid style recalcs.
     if (!m_document->body() || m_document->hasNodesWithPlaceholderStyle())
         return;
-    for (unsigned i = oldStylesheetCount; i < newStylesheetCount; ++i) {
-        if (!newStylesheets[i]->isCSSStyleSheet())
+    for (unsigned i = 0; i < addedSheets.size(); ++i) {
+        if (!addedSheets[i]->isCSSStyleSheet())
             return;
-        if (newStylesheets[i]->disabled())
+        if (addedSheets[i]->disabled())
             continue;
-        if (testAddedStyleSheetRequiresStyleRecalc(static_cast<CSSStyleSheet*>(newStylesheets[i].get())->contents()))
+        if (testAddedStyleSheetRequiresStyleRecalc(static_cast<CSSStyleSheet*>(addedSheets[i])->contents()))
             return;
     }
     requiresFullStyleRecalc = false;
@@ -449,14 +465,21 @@ bool DocumentStyleSheetCollection::updateActiveStyleSheets(UpdateFlag updateFlag
     Vector<RefPtr<StyleSheet> > newStylesheets;
     collectActiveStyleSheets(newStylesheets);
 
-    bool requiresStyleResolverReset;
+    StyleResolverUpdateType styleResolverUpdateType;
     bool requiresFullStyleRecalc;
-    analyzeStyleSheetChange(updateFlag, newStylesheets, requiresStyleResolverReset, requiresFullStyleRecalc);
+    analyzeStyleSheetChange(updateFlag, newStylesheets, styleResolverUpdateType, requiresFullStyleRecalc);
 
-    if (requiresStyleResolverReset)
+    if (styleResolverUpdateType == Reconstruct)
         m_document->clearStyleResolver();
     else {
-        m_document->styleResolver()->appendAuthorStylesheets(m_authorStyleSheets.size(), newStylesheets);
+        StyleResolver* styleResolver = m_document->styleResolver();
+        if (styleResolverUpdateType == Reset) {
+            styleResolver->resetAuthorStyle();
+            styleResolver->appendAuthorStylesheets(0, newStylesheets);
+        } else {
+            ASSERT(styleResolverUpdateType == Additive);
+            styleResolver->appendAuthorStylesheets(m_authorStyleSheets.size(), newStylesheets);
+        }
         resetCSSFeatureFlags();
     }
     m_authorStyleSheets.swap(newStylesheets);
index a8104df..3fb45f9 100644 (file)
@@ -102,7 +102,12 @@ public:
 private:
     void collectActiveStyleSheets(Vector<RefPtr<StyleSheet> >&);
     bool testAddedStyleSheetRequiresStyleRecalc(StyleSheetContents*);
-    void analyzeStyleSheetChange(UpdateFlag, const Vector<RefPtr<StyleSheet> >& newStylesheets, bool& requiresStyleResolverReset, bool& requiresFullStyleRecalc);
+    enum StyleResolverUpdateType {
+        Reconstruct,
+        Reset,
+        Additive
+    };
+    void analyzeStyleSheetChange(UpdateFlag, const Vector<RefPtr<StyleSheet> >& newStylesheets, StyleResolverUpdateType&, bool& requiresFullStyleRecalc);
 
     Document* m_document;