Title: [129644] trunk
Revision
129644
Author
[email protected]
Date
2012-09-26 07:52:19 -0700 (Wed, 26 Sep 2012)

Log Message

Optimize stylesheet insertions
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:

Modified Paths

Added Paths

Diff

Added: trunk/PerformanceTests/CSS/StyleSheetInsert.html (0 => 129644)


--- trunk/PerformanceTests/CSS/StyleSheetInsert.html	                        (rev 0)
+++ trunk/PerformanceTests/CSS/StyleSheetInsert.html	2012-09-26 14:52:19 UTC (rev 129644)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</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>

Modified: trunk/PerformanceTests/ChangeLog (129643 => 129644)


--- trunk/PerformanceTests/ChangeLog	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/PerformanceTests/ChangeLog	2012-09-26 14:52:19 UTC (rev 129644)
@@ -1,3 +1,14 @@
+2012-09-25  Antti Koivisto  <[email protected]>
+
+        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  <[email protected]>
 
         Skip Dromaeo/jslib-modify-jquery.html per bug 95376.

Modified: trunk/Source/WebCore/ChangeLog (129643 => 129644)


--- trunk/Source/WebCore/ChangeLog	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/Source/WebCore/ChangeLog	2012-09-26 14:52:19 UTC (rev 129644)
@@ -1,3 +1,35 @@
+2012-09-25  Antti Koivisto  <[email protected]>
+
+        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  <[email protected]>
 
         Gesture tap highlighting entire first line

Modified: trunk/Source/WebCore/css/StyleResolver.cpp (129643 => 129644)


--- trunk/Source/WebCore/css/StyleResolver.cpp	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/Source/WebCore/css/StyleResolver.cpp	2012-09-26 14:52:19 UTC (rev 129644)
@@ -425,11 +425,7 @@
     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 @@
 }
 #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

Modified: trunk/Source/WebCore/css/StyleResolver.h (129643 => 129644)


--- trunk/Source/WebCore/css/StyleResolver.h	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/Source/WebCore/css/StyleResolver.h	2012-09-26 14:52:19 UTC (rev 129644)
@@ -159,7 +159,8 @@
     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.

Modified: trunk/Source/WebCore/dom/DocumentStyleSheetCollection.cpp (129643 => 129644)


--- trunk/Source/WebCore/dom/DocumentStyleSheetCollection.cpp	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/Source/WebCore/dom/DocumentStyleSheetCollection.cpp	2012-09-26 14:52:19 UTC (rev 129644)
@@ -374,9 +374,9 @@
     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 @@
     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 @@
     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);

Modified: trunk/Source/WebCore/dom/DocumentStyleSheetCollection.h (129643 => 129644)


--- trunk/Source/WebCore/dom/DocumentStyleSheetCollection.h	2012-09-26 14:35:37 UTC (rev 129643)
+++ trunk/Source/WebCore/dom/DocumentStyleSheetCollection.h	2012-09-26 14:52:19 UTC (rev 129644)
@@ -102,7 +102,12 @@
 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;
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to