Changeset 84586 in webkit


Ignore:
Timestamp:
Apr 21, 2011 6:42:11 PM (13 years ago)
Author:
msaboff@apple.com
Message:

2011-04-21 Michael Saboff <​msaboff@apple.com>

Reviewed by Maciej Stachowiak.

Qualified names used for all TagName access, yet namespace usage is rare
​https://bugs.webkit.org/show_bug.cgi?id=58997

The methods getElementsByTagName and getElementsByTagNameNS where
always creating and using QualifiedNames. QualifiedName::init
was consistently in the top 3 routines when running the Dromaeo
DOM-query benchmark. Split out the functionality so that
getElementsByTagName uses just the local name, an implied "*"
namespace and a separate TagNodeListCache keyed by an atomic name
instead of a QualifiedName. Access to elements via
getElementsByTagNameNS that have "*" namespace are forwarded to
getElementsByTagName as well. This provides ~10% speed up in that
Dromaeo test.

No new tests added, existing tests have coverage. The changes are
an optimization of existing functionality.

  • dom/Node.cpp: (WebCore::Node::removeCachedTagNodeList): (WebCore::Node::getElementsByTagName): (WebCore::Node::getElementsByTagNameNS): (WebCore::NodeListsNodeData::invalidateCaches): (WebCore::NodeListsNodeData::isEmpty):
  • dom/Node.h:
  • dom/NodeRareData.h:
  • dom/TagNodeList.cpp: (WebCore::TagNodeList::~TagNodeList):
Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r84584 r84586  
     12011-04-21  Michael Saboff  <msaboff@apple.com>
     2
     3        Reviewed by Maciej Stachowiak.
     4
     5        Qualified names used for all TagName access, yet namespace usage is rare
     6        https://bugs.webkit.org/show_bug.cgi?id=58997
     7
     8        The methods getElementsByTagName and getElementsByTagNameNS where
     9        always creating and using QualifiedNames.  QualifiedName::init
     10        was consistently in the top 3 routines when running the Dromaeo
     11        DOM-query benchmark.  Split out the functionality so that
     12        getElementsByTagName uses just the local name, an implied "*"
     13        namespace and a separate TagNodeListCache keyed by an atomic name
     14        instead of a QualifiedName.  Access to elements via
     15        getElementsByTagNameNS that have "*" namespace are forwarded to
     16        getElementsByTagName as well.  This provides ~10% speed up in that
     17        Dromaeo test.
     18
     19        No new tests added, existing tests have coverage.  The changes are
     20        an optimization of existing functionality.
     21
     22        * dom/Node.cpp:
     23        (WebCore::Node::removeCachedTagNodeList):
     24        (WebCore::Node::getElementsByTagName):
     25        (WebCore::Node::getElementsByTagNameNS):
     26        (WebCore::NodeListsNodeData::invalidateCaches):
     27        (WebCore::NodeListsNodeData::isEmpty):
     28        * dom/Node.h:
     29        * dom/NodeRareData.h:
     30        * dom/TagNodeList.cpp:
     31        (WebCore::TagNodeList::~TagNodeList):
     32
    1332011-04-21  Geoffrey Garen  <ggaren@apple.com>
    234

  • trunk/Source/WebCore/dom/Node.cpp

    r84520 r84586  
    10931093}
    10941094
    1095 void Node::removeCachedTagNodeList(TagNodeList* list, const QualifiedName& name)
     1095void Node::removeCachedTagNodeList(TagNodeList* list, const AtomicString& name)
    10961096{
    10971097    ASSERT(rareData());
    … …  
    11021102    ASSERT_UNUSED(list, list == data->m_tagNodeListCache.get(name.impl()));
    11031103    data->m_tagNodeListCache.remove(name.impl());
     1104}
     1105
     1106void Node::removeCachedTagNodeList(TagNodeList* list, const QualifiedName& name)
     1107{
     1108    ASSERT(rareData());
     1109    ASSERT(rareData()->nodeLists());
     1110    ASSERT_UNUSED(list, list->hasOwnCaches());
     1111
     1112    NodeListsNodeData* data = rareData()->nodeLists();
     1113    ASSERT_UNUSED(list, list == data->m_tagNodeListCacheNS.get(name.impl()));
     1114    data->m_tagNodeListCacheNS.remove(name.impl());
    11041115}
    11051116
    … …  
    17521763// FIXME: End of obviously misplaced HTML editing functions.  Try to move these out of Node.
    17531764
    1754 PassRefPtr<NodeList> Node::getElementsByTagName(const AtomicString& name)
    1755 {
    1756     return getElementsByTagNameNS(starAtom, name);
    1757 }
    1758  
    1759 PassRefPtr<NodeList> Node::getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName)
     1765PassRefPtr<NodeList> Node::getElementsByTagName(const AtomicString& localName)
    17601766{
    17611767    if (localName.isNull())
    17621768        return 0;
    1763    
     1769
    17641770    NodeRareData* data = ensureRareData();
    17651771    if (!data->nodeLists()) {
    … …  
    17711777    if (document()->isHTMLDocument())
    17721778        name = localName.lower();
    1773    
     1779
    17741780    AtomicString localNameAtom = name;
    1775        
    1776     pair<NodeListsNodeData::TagNodeListCache::iterator, bool> result = data->nodeLists()->m_tagNodeListCache.add(QualifiedName(nullAtom, localNameAtom, namespaceURI).impl(), 0);
     1781
     1782    pair<NodeListsNodeData::TagNodeListCache::iterator, bool> result = data->nodeLists()->m_tagNodeListCache.add(localNameAtom, 0);
    17771783    if (!result.second)
    17781784        return PassRefPtr<TagNodeList>(result.first->second);
    1779    
     1785
     1786    RefPtr<TagNodeList> list = TagNodeList::create(this, starAtom, localNameAtom);
     1787    result.first->second = list.get();
     1788    return list.release();   
     1789}
     1790
     1791PassRefPtr<NodeList> Node::getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName)
     1792{
     1793    if (localName.isNull())
     1794        return 0;
     1795
     1796    if (namespaceURI == starAtom)
     1797        return getElementsByTagName(localName);
     1798
     1799    NodeRareData* data = ensureRareData();
     1800    if (!data->nodeLists()) {
     1801        data->setNodeLists(NodeListsNodeData::create());
     1802        document()->addNodeListCache();
     1803    }
     1804
     1805    String name = localName;
     1806    if (document()->isHTMLDocument())
     1807        name = localName.lower();
     1808
     1809    AtomicString localNameAtom = name;
     1810
     1811    pair<NodeListsNodeData::TagNodeListCacheNS::iterator, bool> result = data->nodeLists()->m_tagNodeListCacheNS.add(QualifiedName(nullAtom, localNameAtom, namespaceURI).impl(), 0);
     1812    if (!result.second)
     1813        return PassRefPtr<TagNodeList>(result.first->second);
     1814
    17801815    RefPtr<TagNodeList> list = TagNodeList::create(this, namespaceURI.isEmpty() ? nullAtom : namespaceURI, localNameAtom);
    17811816    result.first->second = list.get();
    … …  
    24582493    for (TagNodeListCache::const_iterator it = m_tagNodeListCache.begin(); it != tagCacheEnd; ++it)
    24592494        it->second->invalidateCache();
     2495    TagNodeListCacheNS::const_iterator tagCacheNSEnd = m_tagNodeListCacheNS.end();
     2496    for (TagNodeListCacheNS::const_iterator it = m_tagNodeListCacheNS.begin(); it != tagCacheNSEnd; ++it)
     2497        it->second->invalidateCache();
    24602498    invalidateCachesThatDependOnAttributes();
    24612499}
    … …  
    24842522    TagNodeListCache::const_iterator tagCacheEnd = m_tagNodeListCache.end();
    24852523    for (TagNodeListCache::const_iterator it = m_tagNodeListCache.begin(); it != tagCacheEnd; ++it) {
     2524        if (it->second->refCount())
     2525            return false;
     2526    }
     2527
     2528    TagNodeListCacheNS::const_iterator tagCacheNSEnd = m_tagNodeListCacheNS.end();
     2529    for (TagNodeListCacheNS::const_iterator it = m_tagNodeListCacheNS.begin(); it != tagCacheNSEnd; ++it) {
    24862530        if (it->second->refCount())
    24872531            return false;

  • trunk/Source/WebCore/dom/Node.h

    r84556 r84586  
    515515    void removeCachedClassNodeList(ClassNodeList*, const String&);
    516516    void removeCachedNameNodeList(NameNodeList*, const String&);
     517    void removeCachedTagNodeList(TagNodeList*, const AtomicString&);
    517518    void removeCachedTagNodeList(TagNodeList*, const QualifiedName&);
    518519    void removeCachedLabelsNodeList(DynamicNodeList*);
    519    
     520
    520521    PassRefPtr<NodeList> getElementsByTagName(const AtomicString&);
    521522    PassRefPtr<NodeList> getElementsByTagNameNS(const AtomicString& namespaceURI, const AtomicString& localName);

  • trunk/Source/WebCore/dom/NodeRareData.h

    r83349 r84586  
    5050    typedef HashMap<String, NameNodeList*> NameNodeListCache;
    5151    NameNodeListCache m_nameNodeListCache;
    52     
    53     typedef HashMap<RefPtr<QualifiedName::QualifiedNameImpl>, TagNodeList*> TagNodeListCache;
     52 
     53    typedef HashMap<AtomicString, TagNodeList*> TagNodeListCache;
    5454    TagNodeListCache m_tagNodeListCache;
    5555
     56    typedef HashMap<RefPtr<QualifiedName::QualifiedNameImpl>, TagNodeList*> TagNodeListCacheNS;
     57    TagNodeListCacheNS m_tagNodeListCacheNS;
     58 
    5659    RefPtr<DynamicNodeList> m_labelsNodeListCache;
    57     
     60 
    5861    static PassOwnPtr<NodeListsNodeData> create()
    5962    {

  • trunk/Source/WebCore/dom/TagNodeList.cpp

    r58526 r84586  
    4040TagNodeList::~TagNodeList()
    4141{
    42     m_rootNode->removeCachedTagNodeList(this, QualifiedName(nullAtom, m_localName, m_namespaceURI));
    43 }
     42    if (m_namespaceURI == starAtom)
     43        m_rootNode->removeCachedTagNodeList(this, m_localName);
     44    else
     45        m_rootNode->removeCachedTagNodeList(this, QualifiedName(nullAtom, m_localName, m_namespaceURI));
     46}
    4447
    4548bool TagNodeList::nodeMatches(Element* testNode) const
Note: See TracChangeset for help on using the changeset viewer.