Attachment 350167 Details for Bug 189775 – Improved the logging

[patch] Improved the logging

bug-189775-20180919205856.patch (text/plain), 11.29 KB, created by Ryosuke Niwa on 2018-09-19 20:58:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-09-19 20:58:57 PDT

Size: 11.29 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 236239)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2018-09-19  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Improve node statistics for rare data
>+        https://bugs.webkit.org/show_bug.cgi?id=189775
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Report reasons we created NodeRareData and ElementRareData in node statistics.
>+
>+        Added NodeRareData::useTypes and ElementRareData::useTypes which returns OptionSet<NodeRareData::UseType>
>+        indicating which instance member of the rare data is currently in use.
>+
>+        * dom/Element.cpp:
>+        * dom/Element.h:
>+        * dom/ElementRareData.h:
>+        (WebCore::defaultMinimumSizeForResizing):
>+        (WebCore::ElementRareData::useTypes const):
>+        * dom/Node.cpp:
>+        (WebCore::stringForRareDataUseType):
>+        (WebCore::Node::dumpStatistics):
>+        * dom/NodeRareData.cpp:
>+        * dom/NodeRareData.h:
>+        (WebCore::NodeRareData::useTypes const):
>+
> 2018-09-19  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         REGRESSION(r235917): 2% regression in Dromaeo CSS selector on MacBookPro11,4
>Index: Source/WebCore/dom/Element.cpp
>===================================================================
>--- Source/WebCore/dom/Element.cpp	(revision 236239)
>+++ Source/WebCore/dom/Element.cpp	(working copy)
>@@ -3442,7 +3442,7 @@ bool Element::fastAttributeLookupAllowed
> }
> #endif
> 
>-#ifdef DUMP_NODE_STATISTICS
>+#if DUMP_NODE_STATISTICS
> bool Element::hasNamedNodeMap() const
> {
>     return hasRareData() && elementRareData()->attributeMap();
>Index: Source/WebCore/dom/Element.h
>===================================================================
>--- Source/WebCore/dom/Element.h	(revision 236239)
>+++ Source/WebCore/dom/Element.h	(working copy)
>@@ -104,7 +104,7 @@ public:
>     WEBCORE_EXPORT bool fastAttributeLookupAllowed(const QualifiedName&) const;
> #endif
> 
>-#ifdef DUMP_NODE_STATISTICS
>+#if DUMP_NODE_STATISTICS
>     bool hasNamedNodeMap() const;
> #endif
>     WEBCORE_EXPORT bool hasAttributes() const;
>Index: Source/WebCore/dom/ElementRareData.h
>===================================================================
>--- Source/WebCore/dom/ElementRareData.h	(revision 236239)
>+++ Source/WebCore/dom/ElementRareData.h	(working copy)
>@@ -33,6 +33,11 @@
> 
> namespace WebCore {
> 
>+inline IntSize defaultMinimumSizeForResizing()
>+{
>+    return IntSize(LayoutUnit::max(), LayoutUnit::max());
>+}
>+
> class ElementRareData : public NodeRareData {
> public:
>     explicit ElementRareData(RenderElement*);
>@@ -122,6 +127,41 @@ public:
>     void setIntersectionObserverData(std::unique_ptr<IntersectionObserverData>&& data) { m_intersectionObserverData = WTFMove(data); }
> #endif
> 
>+#if DUMP_NODE_STATISTICS
>+    OptionSet<UseType> useTypes() const
>+    {
>+        auto result = NodeRareData::useTypes();
>+        if (m_tabIndexWasSetExplicitly)
>+            result.add(UseType::TabIndex);
>+        if (m_styleAffectedByActive || m_styleAffectedByEmpty || m_styleAffectedByFocusWithin || m_childrenAffectedByHover
>+            || m_childrenAffectedByDrag || m_childrenAffectedByLastChildRules || m_childrenAffectedByForwardPositionalRules
>+            || m_descendantsAffectedByForwardPositionalRules || m_childrenAffectedByBackwardPositionalRules
>+            || m_descendantsAffectedByBackwardPositionalRules || m_childrenAffectedByPropertyBasedBackwardPositionalRules)
>+            result.add(UseType::StyleFlags);
>+        if (m_minimumSizeForResizing != defaultMinimumSizeForResizing())
>+            result.add(UseType::MinimumSize);
>+        if (!m_savedLayerScrollPosition.isZero())
>+            result.add(UseType::ScrollingPosition);
>+        if (m_computedStyle)
>+            result.add(UseType::ComputedStyle);
>+        if (m_dataset)
>+            result.add(UseType::Dataset);
>+        if (m_classList)
>+            result.add(UseType::ClassList);
>+        if (m_shadowRoot)
>+            result.add(UseType::ShadowRoot);
>+        if (m_customElementReactionQueue)
>+            result.add(UseType::CustomElementQueue);
>+        if (m_attributeMap)
>+            result.add(UseType::AttributeMap);
>+        if (m_intersectionObserverData)
>+            result.add(UseType::InteractionObserver);
>+        if (m_beforePseudoElement || m_afterPseudoElement)
>+            result.add(UseType::PseudoElements);
>+        return result;
>+    }
>+#endif
>+
> private:
>     int m_tabIndex;
>     unsigned short m_childIndex;
>@@ -165,11 +205,6 @@ private:
>     void releasePseudoElement(PseudoElement*);
> };
> 
>-inline IntSize defaultMinimumSizeForResizing()
>-{
>-    return IntSize(LayoutUnit::max(), LayoutUnit::max());
>-}
>-
> inline ElementRareData::ElementRareData(RenderElement* renderer)
>     : NodeRareData(renderer)
>     , m_tabIndex(0)
>Index: Source/WebCore/dom/Node.cpp
>===================================================================
>--- Source/WebCore/dom/Node.cpp	(revision 236239)
>+++ Source/WebCore/dom/Node.cpp	(working copy)
>@@ -91,6 +91,44 @@ static HashSet<Node*>& liveNodeSet()
>     static NeverDestroyed<HashSet<Node*>> liveNodes;
>     return liveNodes;
> }
>+
>+static const char* stringForRareDataUseType(NodeRareData::UseType useType)
>+{
>+    switch (useType) {
>+    case NodeRareData::UseType::ConnectedFrameCount:
>+        return "ConnectedFrameCount";
>+    case NodeRareData::UseType::NodeList:
>+        return "NodeList";
>+    case NodeRareData::UseType::MutationObserver:
>+        return "MutationObserver";
>+    case NodeRareData::UseType::TabIndex:
>+        return "TabIndex";
>+    case NodeRareData::UseType::StyleFlags:
>+        return "StyleFlags";
>+    case NodeRareData::UseType::MinimumSize:
>+        return "MinimumSize";
>+    case NodeRareData::UseType::ScrollingPosition:
>+        return "ScrollingPosition";
>+    case NodeRareData::UseType::ComputedStyle:
>+        return "ComputedStyle";
>+    case NodeRareData::UseType::Dataset:
>+        return "Dataset";
>+    case NodeRareData::UseType::ClassList:
>+        return "ClassList";
>+    case NodeRareData::UseType::ShadowRoot:
>+        return "ShadowRoot";
>+    case NodeRareData::UseType::CustomElementQueue:
>+        return "CustomElementQueue";
>+    case NodeRareData::UseType::AttributeMap:
>+        return "AttributeMap";
>+    case NodeRareData::UseType::InteractionObserver:
>+        return "InteractionObserver";
>+    case NodeRareData::UseType::PseudoElements:
>+        return "PseudoElements";
>+    }
>+    return nullptr;
>+}
>+
> #endif
> 
> void Node::dumpStatistics()
>@@ -117,6 +155,9 @@ void Node::dumpStatistics()
>     size_t elementsWithRareData = 0;
>     size_t elementsWithNamedNodeMap = 0;
> 
>+    HashMap<uint16_t, size_t> rareDataSingleUseTypeCounts;
>+    size_t mixedRareDataUseCount = 0;
>+
>     for (auto* node : liveNodeSet()) {
>         if (node->hasRareData()) {
>             ++nodesWithRareData;
>@@ -125,6 +166,18 @@ void Node::dumpStatistics()
>                 if (downcast<Element>(*node).hasNamedNodeMap())
>                     ++elementsWithNamedNodeMap;
>             }
>+            auto* rareData = node->rareData();
>+            auto useTypes = is<Element>(node) ? static_cast<ElementRareData*>(rareData)->useTypes() : rareData->useTypes();
>+            unsigned useTypeCount = 0;
>+            for (auto type : useTypes) {
>+                UNUSED_PARAM(type);
>+                useTypeCount++;
>+            }
>+            if (useTypeCount == 1) {
>+                auto result = rareDataSingleUseTypeCounts.add(static_cast<uint16_t>(*useTypes.begin()), 0);
>+                result.iterator->value++;
>+            } else
>+                mixedRareDataUseCount++;
>         }
> 
>         switch (node->nodeType()) {
>@@ -143,7 +196,7 @@ void Node::dumpStatistics()
>                     ++elementsWithAttributeStorage;
>                     for (unsigned i = 0; i < length; ++i) {
>                         const Attribute& attr = elementData->attributeAt(i);
>-                        if (!attr.isEmpty())
>+                        if (element.attrIfExists(attr.name()))
>                             ++attributesWithAttr;
>                     }
>                 }
>@@ -188,7 +241,12 @@ void Node::dumpStatistics()
>     }
> 
>     printf("Number of Nodes: %d\n\n", liveNodeSet().size());
>-    printf("Number of Nodes with RareData: %zu\n\n", nodesWithRareData);
>+    printf("Number of Nodes with RareData: %zu\n", nodesWithRareData);
>+    printf("  Mixed use: %zu\n", mixedRareDataUseCount);
>+    for (auto it : rareDataSingleUseTypeCounts)
>+        printf("  %s: %zu\n", stringForRareDataUseType(static_cast<NodeRareData::UseType>(it.key)), it.value);
>+    printf("\n");
>+
> 
>     printf("NodeType distribution:\n");
>     printf("  Number of Element nodes: %zu\n", elementNodes);
>Index: Source/WebCore/dom/NodeRareData.cpp
>===================================================================
>--- Source/WebCore/dom/NodeRareData.cpp	(revision 236239)
>+++ Source/WebCore/dom/NodeRareData.cpp	(working copy)
>@@ -40,4 +40,7 @@ struct SameSizeAsNodeRareData {
> 
> COMPILE_ASSERT(sizeof(NodeRareData) == sizeof(SameSizeAsNodeRareData), NodeRareDataShouldStaySmall);
> 
>+// Ensure the 10 bits reserved for the m_connectedFrameCount cannot overflow
>+static_assert(Page::maxNumberOfFrames < 1024, "Frame limit should fit in rare data count");
>+
> } // namespace WebCore
>Index: Source/WebCore/dom/NodeRareData.h
>===================================================================
>--- Source/WebCore/dom/NodeRareData.h	(revision 236239)
>+++ Source/WebCore/dom/NodeRareData.h	(working copy)
>@@ -26,7 +26,6 @@
> #include "HTMLNames.h"
> #include "LiveNodeList.h"
> #include "MutationObserverRegistration.h"
>-#include "Page.h"
> #include "QualifiedName.h"
> #include "TagCollection.h"
> #include <wtf/HashSet.h>
>@@ -250,6 +249,27 @@ public:
> class NodeRareData : public NodeRareDataBase {
>     WTF_MAKE_NONCOPYABLE(NodeRareData); WTF_MAKE_FAST_ALLOCATED;
> public:
>+#if defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS
>+    enum class UseType : uint16_t {
>+        ConnectedFrameCount = 1 << 0,
>+        NodeList = 1 << 1,
>+        MutationObserver = 1 << 2,
>+
>+        TabIndex = 1 << 3,
>+        StyleFlags = 1 << 4,
>+        MinimumSize = 1 << 5,
>+        ScrollingPosition = 1 << 6,
>+        ComputedStyle = 1 << 7,
>+        Dataset = 1 << 8,
>+        ClassList = 1 << 9,
>+        ShadowRoot = 1 << 10,
>+        CustomElementQueue = 1 << 11,
>+        AttributeMap = 1 << 12,
>+        InteractionObserver = 1 << 13,
>+        PseudoElements = 1 << 14,
>+    };
>+#endif
>+
>     NodeRareData(RenderObject* renderer)
>         : NodeRareDataBase(renderer)
>         , m_connectedFrameCount(0)
>@@ -284,6 +304,20 @@ public:
>         m_connectedFrameCount -= amount;
>     }
> 
>+#if DUMP_NODE_STATISTICS
>+    OptionSet<UseType> useTypes() const
>+    {
>+        OptionSet<UseType> result;
>+        if (m_connectedFrameCount)
>+            result.add(UseType::ConnectedFrameCount);
>+        if (m_nodeLists)
>+            result.add(UseType::NodeList);
>+        if (m_mutationObserverData)
>+            result.add(UseType::MutationObserver);
>+        return result;
>+    }
>+#endif
>+
> private:
>     unsigned m_connectedFrameCount : 10; // Must fit Page::maxNumberOfFrames.
> 
>@@ -314,7 +348,4 @@ inline NodeRareData& Node::ensureRareDat
>     return *rareData();
> }
> 
>-// Ensure the 10 bits reserved for the m_connectedFrameCount cannot overflow
>-static_assert(Page::maxNumberOfFrames < 1024, "Frame limit should fit in rare data count");
>-
> } // namespace WebCore

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189775: 350167 | 350172