Title: [140070] trunk
Revision
140070
Author
rn...@webkit.org
Date
2013-01-17 16:35:57 -0800 (Thu, 17 Jan 2013)

Log Message

Remove NodeListsNodeData when it's no longer needed
https://bugs.webkit.org/show_bug.cgi?id=107074

Reviewed by Darin Adler.

PerformanceTests: 

Added a micro benchmark to see the benefit of removing NodeListsNodeData.
The test traverses all elements in the html5 specification page and accesses childNodes.

Don't enable this test for now since it's really a micro benchmark specifically
designed to test this patch.

* DOM/TraverseChildNodes.html: Added.
* Skipped: Don't enable newly added test by default.
* resources/results-template.html: Compare against the unscaled unit (e.g. "bytes") as
opposed to scaled units such as "K bytes".
* resources/runner.js:
(.start): Moved the code to call currentTest.setup from measureRunsPerSecondOnce so that
it'll be ran for all test types, namely of PerfTestRunner.measureTime.
(.measureRunsPerSecondOnce):

Source/WebCore: 

Remove NodeListsNodeData when the last node list is removed from it.

If we detect that we have only one node list left in the data structure,
we'll simply destroy the entire "this" object to free up the memory space.

This reduced the memory usage of the micro benchmark by roughly 3%.

Performance Tests: DOM/TraverseChildNodes.html

* dom/Node.cpp:
(WebCore::Node::clearNodeLists): Added.
* dom/Node.h:
* dom/NodeRareData.h:
(WebCore::NodeListsNodeData::removeChildNodeList):
(WebCore::NodeListsNodeData::removeCacheWithAtomicName):
(WebCore::NodeListsNodeData::removeCacheWithName):
(WebCore::NodeListsNodeData::removeCacheWithQualifiedName):
(WebCore::NodeListsNodeData::deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList): Added.
Removes "this" NodeListsNodeData if there is only one node list left.

Tools: 

Generalize the warning a little so that it's also ignored on PerformanceTests/DOM/TraverseChildNodes.html

* Scripts/webkitpy/performance_tests/perftest.py:
(PerfTest):

Modified Paths

Added Paths

Diff

Modified: trunk/PerformanceTests/ChangeLog (140069 => 140070)


--- trunk/PerformanceTests/ChangeLog	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/PerformanceTests/ChangeLog	2013-01-18 00:35:57 UTC (rev 140070)
@@ -1,3 +1,25 @@
+2013-01-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        Remove NodeListsNodeData when it's no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=107074
+
+        Reviewed by Darin Adler.
+
+        Added a micro benchmark to see the benefit of removing NodeListsNodeData.
+        The test traverses all elements in the html5 specification page and accesses childNodes.
+
+        Don't enable this test for now since it's really a micro benchmark specifically
+        designed to test this patch.
+
+        * DOM/TraverseChildNodes.html: Added.
+        * Skipped: Don't enable newly added test by default.
+        * resources/results-template.html: Compare against the unscaled unit (e.g. "bytes") as
+        opposed to scaled units such as "K bytes".
+        * resources/runner.js:
+        (.start): Moved the code to call currentTest.setup from measureRunsPerSecondOnce so that
+        it'll be ran for all test types, namely of PerfTestRunner.measureTime.
+        (.measureRunsPerSecondOnce):
+
 2013-01-17  Eric Seidel  <e...@webkit.org>
 
         Add a version of the html-parser benchmark which uses srcdoc instead of document.write so it tests the threaded parser

Added: trunk/PerformanceTests/DOM/TraverseChildNodes.html (0 => 140070)


--- trunk/PerformanceTests/DOM/TraverseChildNodes.html	                        (rev 0)
+++ trunk/PerformanceTests/DOM/TraverseChildNodes.html	2013-01-18 00:35:57 UTC (rev 140070)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+var spec = PerfTestRunner.loadFile("../Parser/resources/html5.html");
+var iframe;
+
+PerfTestRunner.measureTime({
+    setup: function () {
+        if (iframe)
+            document.body.removeChild(iframe);
+        iframe = document.createElement("iframe");
+        iframe.style.display = "none";  // Prevent creation of the rendering tree, so we only test HTML parsing.
+        iframe.sandbox = '';  // Prevent external script loads which could cause write() to return before completing the parse.
+        document.body.appendChild(iframe);
+        iframe.contentDocument.open();
+        iframe.contentDocument.write(spec);
+        iframe.contentDocument.close();
+    },
+    run: function() {
+        var elements = iframe.contentDocument.getElementsByTagName('*');
+        for (var i = 0; i < elements.length; i++) {
+            for (var j = 0; j < elements[i].childNodes.length; j++)
+                elements[i].childNodes[j];
+        }
+    },
+    done: function () { document.body.removeChild(iframe); }});
+
+</script>
+</body>
+</html>

Modified: trunk/PerformanceTests/Skipped (140069 => 140070)


--- trunk/PerformanceTests/Skipped	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/PerformanceTests/Skipped	2013-01-18 00:35:57 UTC (rev 140070)
@@ -1,3 +1,6 @@
+# Micro benchmarks not worth running at the moment.
+DOM/child-node-traversal.html
+
 # Not enabled by default on some ports
 Mutation
 

Modified: trunk/PerformanceTests/resources/results-template.html (140069 => 140070)


--- trunk/PerformanceTests/resources/results-template.html	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/PerformanceTests/resources/results-template.html	2013-01-18 00:35:57 UTC (rev 140070)
@@ -255,7 +255,7 @@
         computeScalingFactorIfNeeded();
         return cachedUnit;
     }
-    this.smallerIsBetter = function () { return this.unit() == 'ms' || this.unit() == 'bytes'; }
+    this.smallerIsBetter = function () { return testResults[0].unit() == 'ms' || testResults[0].unit() == 'bytes'; }
 }
 
 var plotColor = 'rgb(230,50,50)';

Modified: trunk/PerformanceTests/resources/runner.js (140069 => 140070)


--- trunk/PerformanceTests/resources/runner.js	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/PerformanceTests/resources/runner.js	2013-01-18 00:35:57 UTC (rev 140070)
@@ -166,6 +166,9 @@
         PerfTestRunner.gc();
         window.setTimeout(function () {
             try {
+                if (currentTest.setup)
+                    currentTest.setup();
+
                 var measuredValue = runner();
             } catch (exception) {
                 logFatalError("Got an exception while running test.run with name=" + exception.name + ", message=" + exception.message);
@@ -275,9 +278,6 @@
         var totalTime = 0;
         var numberOfRuns = 0;
 
-        if (currentTest.setup)
-            currentTest.setup();
-
         while (totalTime < timeToRun) {
             totalTime += callRunAndMeasureTime(callsPerIteration);
             numberOfRuns += callsPerIteration;

Modified: trunk/Source/WebCore/ChangeLog (140069 => 140070)


--- trunk/Source/WebCore/ChangeLog	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Source/WebCore/ChangeLog	2013-01-18 00:35:57 UTC (rev 140070)
@@ -1,3 +1,30 @@
+2013-01-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        Remove NodeListsNodeData when it's no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=107074
+
+        Reviewed by Darin Adler.
+
+        Remove NodeListsNodeData when the last node list is removed from it.
+
+        If we detect that we have only one node list left in the data structure,
+        we'll simply destroy the entire "this" object to free up the memory space.
+
+        This reduced the memory usage of the micro benchmark by roughly 3%.
+
+        Performance Tests: DOM/TraverseChildNodes.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::clearNodeLists): Added.
+        * dom/Node.h:
+        * dom/NodeRareData.h:
+        (WebCore::NodeListsNodeData::removeChildNodeList):
+        (WebCore::NodeListsNodeData::removeCacheWithAtomicName):
+        (WebCore::NodeListsNodeData::removeCacheWithName):
+        (WebCore::NodeListsNodeData::removeCacheWithQualifiedName):
+        (WebCore::NodeListsNodeData::deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList): Added.
+        Removes "this" NodeListsNodeData if there is only one node list left.
+
 2013-01-17  Abhishek Arya  <infe...@chromium.org>
 
         Heap-use-after-free in WebCore::RenderBlock::checkFloatsInCleanLine

Modified: trunk/Source/WebCore/dom/Node.cpp (140069 => 140070)


--- trunk/Source/WebCore/dom/Node.cpp	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Source/WebCore/dom/Node.cpp	2013-01-18 00:35:57 UTC (rev 140070)
@@ -981,6 +981,11 @@
     return hasRareData() ? rareData()->nodeLists() : 0;
 }
 
+void Node::clearNodeLists()
+{
+    rareData()->clearNodeLists();
+}
+
 void Node::checkSetPrefix(const AtomicString& prefix, ExceptionCode& ec)
 {
     // Perform error checking as required by spec for setting Node.prefix. Used by

Modified: trunk/Source/WebCore/dom/Node.h (140069 => 140070)


--- trunk/Source/WebCore/dom/Node.h	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Source/WebCore/dom/Node.h	2013-01-18 00:35:57 UTC (rev 140070)
@@ -582,6 +582,7 @@
 
     void invalidateNodeListCachesInAncestors(const QualifiedName* attrName = 0, Element* attributeOwnerElement = 0);
     NodeListsNodeData* nodeLists();
+    void clearNodeLists();
 
     PassRefPtr<NodeList> getElementsByTagName(const AtomicString&);
     PassRefPtr<NodeList> getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName);

Modified: trunk/Source/WebCore/dom/NodeRareData.h (140069 => 140070)


--- trunk/Source/WebCore/dom/NodeRareData.h	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Source/WebCore/dom/NodeRareData.h	2013-01-18 00:35:57 UTC (rev 140070)
@@ -69,7 +69,9 @@
 
     void removeChildNodeList(ChildNodeList* list)
     {
-        ASSERT_UNUSED(list, m_childNodeList = list);
+        ASSERT(m_childNodeList = list);
+        if (deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(list->ownerNode()))
+            return;
         m_childNodeList = 0;
     }
 
@@ -144,20 +146,26 @@
 
     void removeCacheWithAtomicName(LiveNodeListBase* list, CollectionType collectionType, const AtomicString& name = starAtom)
     {
-        ASSERT_UNUSED(list, list == m_atomicNameCaches.get(namedNodeListKey(collectionType, name)));
+        ASSERT(list == m_atomicNameCaches.get(namedNodeListKey(collectionType, name)));
+        if (deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(list->ownerNode()))
+            return;
         m_atomicNameCaches.remove(namedNodeListKey(collectionType, name));
     }
 
     void removeCacheWithName(LiveNodeListBase* list, CollectionType collectionType, const String& name)
     {
-        ASSERT_UNUSED(list, list == m_nameCaches.get(namedNodeListKey(collectionType, name)));
+        ASSERT(list == m_nameCaches.get(namedNodeListKey(collectionType, name)));
+        if (deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(list->ownerNode()))
+            return;
         m_nameCaches.remove(namedNodeListKey(collectionType, name));
     }
 
     void removeCacheWithQualifiedName(LiveNodeList* list, const AtomicString& namespaceURI, const AtomicString& localName)
     {
         QualifiedName name(nullAtom, localName, namespaceURI);
-        ASSERT_UNUSED(list, list == m_tagNodeListCacheNS.get(name));
+        ASSERT(list == m_tagNodeListCacheNS.get(name));
+        if (deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(list->ownerNode()))
+            return;
         m_tagNodeListCacheNS.remove(name);
     }
 
@@ -218,6 +226,8 @@
         return std::pair<unsigned char, String>(type, name);
     }
 
+    bool deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(Node*);
+
     // FIXME: m_childNodeList should be merged into m_atomicNameCaches or at least be shared with HTMLCollection returned by Element::children
     // but it's tricky because invalidateCaches shouldn't invalidate this cache and adoptTreeScope shouldn't call registerNodeList or unregisterNodeList.
     ChildNodeList* m_childNodeList;
@@ -322,6 +332,16 @@
 #endif
 };
 
+inline bool NodeListsNodeData::deleteThisAndUpdateNodeRareDataIfAboutToRemoveLastList(Node* ownerNode)
+{
+    ASSERT(ownerNode);
+    ASSERT(ownerNode->nodeLists() == this);
+    if ((m_childNodeList ? 1 : 0) + m_atomicNameCaches.size() + m_nameCaches.size() + m_tagNodeListCacheNS.size() != 1)
+        return false;
+    ownerNode->clearNodeLists();
+    return true;
+}
+
 } // namespace WebCore
 
 #endif // NodeRareData_h

Modified: trunk/Tools/ChangeLog (140069 => 140070)


--- trunk/Tools/ChangeLog	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Tools/ChangeLog	2013-01-18 00:35:57 UTC (rev 140070)
@@ -1,3 +1,15 @@
+2013-01-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        Remove NodeListsNodeData when it's no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=107074
+
+        Reviewed by Darin Adler.
+
+        Generalize the warning a little so that it's also ignored on PerformanceTests/DOM/TraverseChildNodes.html
+
+        * Scripts/webkitpy/performance_tests/perftest.py:
+        (PerfTest):
+
 2013-01-17  Simon Fraser  <simon.fra...@apple.com>
 
         Ref test images are upside-down in WebKit2

Modified: trunk/Tools/Scripts/webkitpy/performance_tests/perftest.py (140069 => 140070)


--- trunk/Tools/Scripts/webkitpy/performance_tests/perftest.py	2013-01-18 00:22:41 UTC (rev 140069)
+++ trunk/Tools/Scripts/webkitpy/performance_tests/perftest.py	2013-01-18 00:35:57 UTC (rev 140070)
@@ -205,8 +205,7 @@
         re.compile(re.escape("""frame "<!--framePath //<!--frame0-->/<!--frame0-->-->" - has 1 onunload handler(s)""")),
         # Following is for html5.html
         re.compile(re.escape("""Blocked access to external URL http://www.whatwg.org/specs/web-apps/current-work/""")),
-        # Following is for Parser/html-parser.html
-        re.compile(re.escape("""CONSOLE MESSAGE: Blocked script execution in 'html-parser.html' because the document's frame is sandboxed and the 'allow-scripts' permission is not set.""")),
+        re.compile(r"CONSOLE MESSAGE: Blocked script execution in '[A-Za-z0-9\-\.]+' because the document's frame is sandboxed and the 'allow-scripts' permission is not set."),
         # Dromaeo reports values for subtests. Ignore them for now.
         re.compile(r'(?P<name>.+): \[(?P<values>(\d+(.\d+)?,\s+)*\d+(.\d+)?)\]'),
     ]
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to