Title: [166347] trunk/Source/WebCore
Revision
166347
Author
rn...@webkit.org
Date
2014-03-26 23:22:48 -0700 (Wed, 26 Mar 2014)

Log Message

HTMLConverter::_processText is slow because it walks up ancestor elements
https://bugs.webkit.org/show_bug.cgi?id=130820

Reviewed by Sam Weinig.

Avoid walking up the tree from each text node by caching the aggregated attributed strings for each element.
Also compute the attributed strings top-down to avoid calling mutableCopy in every iteration.

This reduces the runtime of Interactive/CopyAll.html from 15s to 13s (15%).

* editing/cocoa/HTMLConverter.mm:
(HTMLConverter::_attributesForElement):
(HTMLConverter::attributesForElement):
(HTMLConverter::aggregatedAttributesForAncestors):
(HTMLConverter::_processText):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (166346 => 166347)


--- trunk/Source/WebCore/ChangeLog	2014-03-27 06:13:33 UTC (rev 166346)
+++ trunk/Source/WebCore/ChangeLog	2014-03-27 06:22:48 UTC (rev 166347)
@@ -1,3 +1,21 @@
+2014-03-26  Ryosuke Niwa  <rn...@webkit.org>
+
+        HTMLConverter::_processText is slow because it walks up ancestor elements
+        https://bugs.webkit.org/show_bug.cgi?id=130820
+
+        Reviewed by Sam Weinig.
+
+        Avoid walking up the tree from each text node by caching the aggregated attributed strings for each element.
+        Also compute the attributed strings top-down to avoid calling mutableCopy in every iteration. 
+
+        This reduces the runtime of Interactive/CopyAll.html from 15s to 13s (15%).
+
+        * editing/cocoa/HTMLConverter.mm:
+        (HTMLConverter::_attributesForElement):
+        (HTMLConverter::attributesForElement):
+        (HTMLConverter::aggregatedAttributesForAncestors):
+        (HTMLConverter::_processText):
+
 2014-03-26  Sam Weinig  <s...@webkit.org>
 
         Fix iOS build.

Modified: trunk/Source/WebCore/editing/cocoa/HTMLConverter.mm (166346 => 166347)


--- trunk/Source/WebCore/editing/cocoa/HTMLConverter.mm	2014-03-27 06:13:33 UTC (rev 166346)
+++ trunk/Source/WebCore/editing/cocoa/HTMLConverter.mm	2014-03-27 06:22:48 UTC (rev 166347)
@@ -454,7 +454,8 @@
     
 private:
     HashMap<RefPtr<Element>, RetainPtr<NSDictionary>> m_attributesForElements;
-    
+    HashMap<RefPtr<Element>, RetainPtr<NSMutableDictionary>> m_aggregatedAttributesForElements;
+
     NSMutableAttributedString *_attrStr;
     NSMutableDictionary *_documentAttrs;
     NSURL *_baseURL;
@@ -497,6 +498,8 @@
     
     NSDictionary *_computedAttributesForElement(Element&);
     NSDictionary *_attributesForElement(DOMElement *);
+    NSDictionary* attributesForElement(Element& element);
+    NSDictionary* aggregatedAttributesForAncestors(CharacterData&);
     
     Element* _blockLevelElementForNode(Node*);
     
@@ -1352,15 +1355,45 @@
 {
     if (!element)
         return [NSDictionary dictionary];
-    
-    Element& coreElement = *core(element);
-    
-    auto& attributes = m_attributesForElements.add(&coreElement, nullptr).iterator->value;
+    return attributesForElement(*core(element));
+}
+
+NSDictionary* HTMLConverter::attributesForElement(Element& element)
+{
+    auto& attributes = m_attributesForElements.add(&element, nullptr).iterator->value;
     if (!attributes)
-        attributes = _computedAttributesForElement(coreElement);
+        attributes = _computedAttributesForElement(element);
     return attributes.get();
 }
 
+NSDictionary* HTMLConverter::aggregatedAttributesForAncestors(CharacterData& node)
+{
+    Node* ancestor = node.parentNode();
+    while (ancestor && !ancestor->isElementNode())
+        ancestor = ancestor->parentNode();
+    if (!ancestor)
+        return nullptr;
+
+    auto& attributes = m_aggregatedAttributesForElements.add(toElement(ancestor), nullptr).iterator->value;
+    if (!attributes) {
+        Vector<Element*, 16> ancestorElements;
+        ancestorElements.append(toElement(ancestor));
+        for (; ancestor; ancestor = ancestor->parentNode()) {
+            if (ancestor->isElementNode())
+                ancestorElements.append(toElement(ancestor));
+        }
+
+        attributes = [NSMutableDictionary dictionary];
+        // Fill attrs dictionary with attributes from the highest to the lowest, not overwriting ones deeper in the tree
+        for (unsigned index = ancestorElements.size(); index;) {
+            index--;
+            [attributes addEntriesFromDictionary:attributesForElement(*ancestorElements[index])];
+        }
+    }
+
+    return attributes.get();
+}
+
 void HTMLConverter::_newParagraphForElement(DOMElement *element, NSString *tag, BOOL flag, BOOL suppressTrailingSpace)
 {
     NSUInteger textLength = [_attrStr length];
@@ -2332,22 +2365,8 @@
 
         [_attrStr replaceCharactersInRange:rangeToReplace withString:outputString];
         rangeToReplace.length = outputString.length();
-        RetainPtr<NSDictionary> attrs;
-        Node* ancestor = characterData.parentNode();
-        while (ancestor) {
-            // Fill attrs dictionary with attributes from parent nodes, not overwriting ones deeper in the tree
-            if(ancestor->isElementNode()) {
-                RetainPtr<NSMutableDictionary> newAttrs = adoptNS([_attributesForElement(kit(toElement(ancestor))) mutableCopy]);
-                if (attrs) {
-                    // Already-set attributes (from lower in the tree) overwrite the higher ones.
-                    [newAttrs addEntriesFromDictionary:attrs.get()];
-                }
-                attrs = newAttrs;
-            }
-            ancestor = ancestor->parentNode();
-        }
-        if (rangeToReplace.length > 0)
-            [_attrStr setAttributes:attrs.get() range:rangeToReplace];
+        if (rangeToReplace.length)
+            [_attrStr setAttributes:aggregatedAttributesForAncestors(characterData) range:rangeToReplace];
         _flags.isSoft = wasSpace;
     }
     if (mutstr)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to