Title: [122531] trunk/Source/WebCore
Revision
122531
Author
rn...@webkit.org
Date
2012-07-12 17:20:46 -0700 (Thu, 12 Jul 2012)

Log Message

Move m_type and m_hasNameCache from HTMLCollectionCacheBase to DynamicNodeListCacheBase for better bit packing
https://bugs.webkit.org/show_bug.cgi?id=91164

Reviewed by Anders Carlsson.

Moved m_type and m_hasNameCache from HTMLCollection and renamed them to m_collectionType and m_isNameCacheValid.

Also renamed shouldIncludeChildren to shouldOnlyIncludeDirectChildren and negated the return value since
all HTMLCollection include children in the collection and the function was meant to tell us whether the collection
should include descendents that are not direct children of base().

In addition, renamed nextNodeOrSibling to nextNode since "or sibling" doesn't seem to add any semantic clarity.

* dom/DynamicNodeList.h:
(WebCore::DynamicNodeListCacheBase::DynamicNodeListCacheBase):
(DynamicNodeListCacheBase):
(WebCore::DynamicNodeListCacheBase::type):
(WebCore::DynamicNodeListCacheBase::hasNameCache):
(WebCore::DynamicNodeListCacheBase::setHasNameCache):
(WebCore::DynamicNodeListCacheBase::clearCache):
* html/HTMLCollection.cpp:
(WebCore::shouldOnlyIncludeDirectChildren):
(WebCore::HTMLCollection::HTMLCollection):
(WebCore::HTMLCollection::isAcceptableElement):
(WebCore::nextNode):
(WebCore::HTMLCollection::itemAfter):
* html/HTMLCollection.h:
(WebCore::HTMLCollectionCacheBase::HTMLCollectionCacheBase):
(HTMLCollectionCacheBase):
(WebCore::HTMLCollectionCacheBase::clearCache):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (122530 => 122531)


--- trunk/Source/WebCore/ChangeLog	2012-07-13 00:09:21 UTC (rev 122530)
+++ trunk/Source/WebCore/ChangeLog	2012-07-13 00:20:46 UTC (rev 122531)
@@ -1,3 +1,36 @@
+2012-07-12  Ryosuke Niwa  <rn...@webkit.org>
+
+        Move m_type and m_hasNameCache from HTMLCollectionCacheBase to DynamicNodeListCacheBase for better bit packing
+        https://bugs.webkit.org/show_bug.cgi?id=91164
+
+        Reviewed by Anders Carlsson.
+
+        Moved m_type and m_hasNameCache from HTMLCollection and renamed them to m_collectionType and m_isNameCacheValid.
+
+        Also renamed shouldIncludeChildren to shouldOnlyIncludeDirectChildren and negated the return value since
+        all HTMLCollection include children in the collection and the function was meant to tell us whether the collection
+        should include descendents that are not direct children of base().
+
+        In addition, renamed nextNodeOrSibling to nextNode since "or sibling" doesn't seem to add any semantic clarity.
+
+        * dom/DynamicNodeList.h:
+        (WebCore::DynamicNodeListCacheBase::DynamicNodeListCacheBase):
+        (DynamicNodeListCacheBase):
+        (WebCore::DynamicNodeListCacheBase::type):
+        (WebCore::DynamicNodeListCacheBase::hasNameCache):
+        (WebCore::DynamicNodeListCacheBase::setHasNameCache):
+        (WebCore::DynamicNodeListCacheBase::clearCache):
+        * html/HTMLCollection.cpp:
+        (WebCore::shouldOnlyIncludeDirectChildren):
+        (WebCore::HTMLCollection::HTMLCollection):
+        (WebCore::HTMLCollection::isAcceptableElement):
+        (WebCore::nextNode):
+        (WebCore::HTMLCollection::itemAfter):
+        * html/HTMLCollection.h:
+        (WebCore::HTMLCollectionCacheBase::HTMLCollectionCacheBase):
+        (HTMLCollectionCacheBase):
+        (WebCore::HTMLCollectionCacheBase::clearCache):
+
 2012-07-12  Shinya Kawanaka  <shin...@chromium.org>
 
         Needs callback before AuthorShadowRoot is added.

Modified: trunk/Source/WebCore/dom/DynamicNodeList.h (122530 => 122531)


--- trunk/Source/WebCore/dom/DynamicNodeList.h	2012-07-13 00:09:21 UTC (rev 122530)
+++ trunk/Source/WebCore/dom/DynamicNodeList.h	2012-07-13 00:20:46 UTC (rev 122531)
@@ -24,6 +24,7 @@
 #ifndef DynamicNodeList_h
 #define DynamicNodeList_h
 
+#include "CollectionType.h"
 #include "Document.h"
 #include "NodeList.h"
 #include <wtf/Forward.h>
@@ -39,16 +40,25 @@
     DynamicNodeListCacheBase(NodeListRootType rootType, NodeListInvalidationType invalidationType)
         : m_rootedAtDocument(rootType == NodeListIsRootedAtDocument)
         , m_invalidationType(invalidationType)
+        , m_collectionType(InvalidCollectionType)
     {
         ASSERT(m_invalidationType == static_cast<unsigned>(invalidationType));
         clearCache();
     }
 
+    DynamicNodeListCacheBase(CollectionType collectionType)
+        : m_collectionType(collectionType)
+    {
+        ASSERT(m_collectionType == static_cast<unsigned>(collectionType));
+        clearCache();
+    }
+
 public:
     ALWAYS_INLINE bool isRootedAtDocument() const { return m_rootedAtDocument; }
     ALWAYS_INLINE bool shouldInvalidateOnAttributeChange() const { return m_invalidationType != DoNotInvalidateOnAttributeChanges; }
     ALWAYS_INLINE NodeListRootType rootType() { return m_rootedAtDocument ? NodeListIsRootedAtDocument : NodeListIsRootedAtNode; }
     ALWAYS_INLINE NodeListInvalidationType invalidationType() const { return static_cast<NodeListInvalidationType>(m_invalidationType); }
+    ALWAYS_INLINE CollectionType type() const { return static_cast<CollectionType>(m_collectionType); }
 
 protected:
     ALWAYS_INLINE bool isItemCacheValid() const { return m_isItemCacheValid; }
@@ -70,11 +80,15 @@
         m_isItemCacheValid = true;
     }
 
+    bool hasNameCache() const { return m_isNameCacheValid; }
+    void setHasNameCache() const { m_isNameCacheValid = true; }
+
     void clearCache() const
     {
         m_cachedItem = 0;
         m_isLengthCacheValid = false;
         m_isItemCacheValid = false;
+        m_isNameCacheValid = false;
     }
 
 private:
@@ -87,6 +101,10 @@
     // From DynamicNodeList
     const unsigned m_rootedAtDocument : 1;
     const unsigned m_invalidationType : 3;
+
+    // From HTMLCollection
+    mutable unsigned m_isNameCacheValid : 1;
+    const unsigned m_collectionType : 5;
 };
 
 class DynamicNodeList : public NodeList, public DynamicNodeListCacheBase {

Modified: trunk/Source/WebCore/html/CollectionType.h (122530 => 122531)


--- trunk/Source/WebCore/html/CollectionType.h	2012-07-13 00:09:21 UTC (rev 122530)
+++ trunk/Source/WebCore/html/CollectionType.h	2012-07-13 00:20:46 UTC (rev 122531)
@@ -60,7 +60,8 @@
     ItemProperties, // Microdata item properties in the document
 #endif
 
-    FormControls
+    FormControls,
+    InvalidCollectionType
 };
 
 static const CollectionType FirstUnnamedDocumentCachedType = DocImages;

Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (122530 => 122531)


--- trunk/Source/WebCore/html/HTMLCollection.cpp	2012-07-13 00:09:21 UTC (rev 122530)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp	2012-07-13 00:20:46 UTC (rev 122531)
@@ -40,7 +40,7 @@
 
 using namespace HTMLNames;
 
-static bool shouldIncludeChildren(CollectionType type)
+static bool shouldOnlyIncludeDirectChildren(CollectionType type)
 {
     switch (type) {
     case DocAll:
@@ -63,19 +63,21 @@
     case ItemProperties:
 #endif
     case FormControls:
-        return true;
+        return false;
     case NodeChildren:
     case TRCells:
     case TSectionRows:
     case TableTBodies:
-        return false;
+        return true;
+    case InvalidCollectionType:
+        break;
     }
     ASSERT_NOT_REACHED();
     return false;
 }
 
 HTMLCollection::HTMLCollection(Node* base, CollectionType type)
-    : HTMLCollectionCacheBase(type, shouldIncludeChildren(type))
+    : HTMLCollectionCacheBase(type)
     , m_base(base)
 {
     ASSERT(m_base);
@@ -170,31 +172,27 @@
     case DocumentNamedItems:
     case TableRows:
     case WindowNamedItems:
+    case InvalidCollectionType:
         ASSERT_NOT_REACHED();
     }
     return false;
 }
 
-static Node* nextNodeOrSibling(Node* base, Node* node, bool includeChildren)
+static ALWAYS_INLINE Node* nextNode(Node* base, Node* previous, bool onlyIncludeDirectChildren)
 {
-    return includeChildren ? node->traverseNextNode(base) : node->traverseNextSibling(base);
+    return onlyIncludeDirectChildren ? previous->traverseNextSibling(base) : previous->traverseNextNode(base);
 }
 
 Element* HTMLCollection::itemAfter(unsigned& offsetInArray, Element* previous) const
 {
     ASSERT_UNUSED(offsetInArray, !offsetInArray);
-    Node* current;
-    if (!previous)
-        current = m_base->firstChild();
-    else
-        current = nextNodeOrSibling(base(), previous, includeChildren());
+    bool _onlyIncludeDirectChildren_ = shouldOnlyIncludeDirectChildren(type());
+    Node* rootNode = base();
+    Node* current = previous ? nextNode(rootNode, previous, onlyIncludeDirectChildren) : m_base->firstChild();
 
-    for (; current; current = nextNodeOrSibling(base(), current, includeChildren())) {
-        if (!current->isElementNode())
-            continue;
-        Element* element = static_cast<Element*>(current);
-        if (isAcceptableElement(element))
-            return element;
+    for (; current; current = nextNode(rootNode, current, onlyIncludeDirectChildren)) {
+        if (current->isElementNode() && isAcceptableElement(toElement(current)))
+            return toElement(current);
     }
 
     return 0;

Modified: trunk/Source/WebCore/html/HTMLCollection.h (122530 => 122531)


--- trunk/Source/WebCore/html/HTMLCollection.h	2012-07-13 00:09:21 UTC (rev 122530)
+++ trunk/Source/WebCore/html/HTMLCollection.h	2012-07-13 00:20:46 UTC (rev 122531)
@@ -39,19 +39,13 @@
 
 class HTMLCollectionCacheBase : public DynamicNodeListCacheBase {
 public:
-    HTMLCollectionCacheBase(CollectionType type, bool includeChildren)
-        : DynamicNodeListCacheBase(NodeListIsRootedAtNode, DoNotInvalidateOnAttributeChanges) // These two flags are never used
+    HTMLCollectionCacheBase(CollectionType type)
+        : DynamicNodeListCacheBase(type)
         , m_cachedElementsArrayOffset(0)
         , m_cacheTreeVersion(0)
-        , m_hasNameCache(false)
-        , m_type(type)
-        , m_includeChildren(includeChildren)
     {
-        ASSERT(static_cast<CollectionType>(m_type) == type);
     }
 
-    CollectionType type() const { return static_cast<CollectionType>(m_type); }
-
 protected:
     void clearCache(uint64_t currentDomTreeVersion) const
     {
@@ -60,7 +54,6 @@
         m_nameCache.clear();
         m_cachedElementsArrayOffset = 0;
         m_cacheTreeVersion = currentDomTreeVersion;
-        m_hasNameCache = false;
     }
 
     void setItemCache(Node* item, unsigned offset, unsigned elementsArrayOffset) const
@@ -70,7 +63,6 @@
     }
     unsigned cachedElementsArrayOffset() const { return m_cachedElementsArrayOffset; }
 
-    bool includeChildren() const { return m_includeChildren; }
     uint64_t cacheTreeVersion() const { return m_cacheTreeVersion; }
 
     typedef HashMap<AtomicStringImpl*, OwnPtr<Vector<Element*> > > NodeCacheMap;
@@ -79,9 +71,6 @@
     void appendIdCache(const AtomicString& name, Element* element) const { append(m_idCache, name, element); }
     void appendNameCache(const AtomicString& name, Element* element) const { append(m_nameCache, name, element); }
 
-    bool hasNameCache() const { return m_hasNameCache; }
-    void setHasNameCache() const { m_hasNameCache = true; }
-
     static void append(NodeCacheMap&, const AtomicString&, Element*);
 
 private:
@@ -94,11 +83,6 @@
     mutable NodeCacheMap m_nameCache;
     mutable unsigned m_cachedElementsArrayOffset;
     mutable uint64_t m_cacheTreeVersion;
-
-    // FIXME: Move these bit flags to DynamicNodeListCacheBase to pack them better.
-    mutable unsigned m_hasNameCache : 1;
-    const unsigned m_type : 5; // CollectionType
-    const unsigned m_includeChildren : 1;
 };
 
 class HTMLCollection : public RefCounted<HTMLCollection>, public HTMLCollectionCacheBase {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to