2010-07-07 Eric Seidel <eric@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Jul 2010 17:11:28 +0000 (17:11 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Jul 2010 17:11:28 +0000 (17:11 +0000)
        Reviewed by Adam Barth.

        HTMLTreeBuilder is way too slow
        https://bugs.webkit.org/show_bug.cgi?id=41754

        This takes us from 14s to 7s on our parsing benchmark.
        That's still much slower than the old tree builder, but there
        is a huge amount of fat left to trim.

        Vector<T> wasn't able to inline all the Entry functions when
        they were buried in the cpp.  Turns out the active formatting elements
        list is very hot.

        I'm not sure Vector<T> is going to be the right data structure for us
        in the end, but it has done alright for bring-up.

        * html/HTMLFormattingElementList.cpp:
        * html/HTMLFormattingElementList.h:
        (WebCore::HTMLFormattingElementList::Entry::Entry):
        (WebCore::HTMLFormattingElementList::Entry::~Entry):
        (WebCore::HTMLFormattingElementList::Entry::isMarker):
        (WebCore::HTMLFormattingElementList::Entry::element):
        (WebCore::HTMLFormattingElementList::Entry::replaceElement):
        (WebCore::HTMLFormattingElementList::Entry::operator==):
        (WebCore::HTMLFormattingElementList::Entry::operator!=):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@62678 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/html/HTMLFormattingElementList.cpp
WebCore/html/HTMLFormattingElementList.h

index 0825d6a..179b1dd 100644 (file)
@@ -1,3 +1,31 @@
+2010-07-07  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        HTMLTreeBuilder is way too slow
+        https://bugs.webkit.org/show_bug.cgi?id=41754
+
+        This takes us from 14s to 7s on our parsing benchmark.
+        That's still much slower than the old tree builder, but there
+        is a huge amount of fat left to trim.
+
+        Vector<T> wasn't able to inline all the Entry functions when
+        they were buried in the cpp.  Turns out the active formatting elements
+        list is very hot.
+
+        I'm not sure Vector<T> is going to be the right data structure for us
+        in the end, but it has done alright for bring-up.
+
+        * html/HTMLFormattingElementList.cpp:
+        * html/HTMLFormattingElementList.h:
+        (WebCore::HTMLFormattingElementList::Entry::Entry):
+        (WebCore::HTMLFormattingElementList::Entry::~Entry):
+        (WebCore::HTMLFormattingElementList::Entry::isMarker):
+        (WebCore::HTMLFormattingElementList::Entry::element):
+        (WebCore::HTMLFormattingElementList::Entry::replaceElement):
+        (WebCore::HTMLFormattingElementList::Entry::operator==):
+        (WebCore::HTMLFormattingElementList::Entry::operator!=):
+
 2010-07-06  Darin Adler  <darin@apple.com>
 
         Reviewed by Adam Barth.
index e046b51..b7d4ab1 100644 (file)
 
 namespace WebCore {
 
-HTMLFormattingElementList::Entry::Entry(Element* element)
-    : m_element(element)
-{
-    ASSERT(element);
-}
-
-HTMLFormattingElementList::Entry::Entry(MarkerEntryType)
-{
-}
-
-HTMLFormattingElementList::Entry::~Entry()
-{
-}
-
-bool HTMLFormattingElementList::Entry::isMarker() const
-{
-    return !m_element;
-}
-
-Element* HTMLFormattingElementList::Entry::element() const
-{
-    // The fact that !m_element == isMarker() is an implementation detail
-    // callers should check isMarker() before calling element().
-    ASSERT(m_element);
-    return m_element.get();
-}
-
-void HTMLFormattingElementList::Entry::replaceElement(PassRefPtr<Element> element)
-{
-    ASSERT(m_element); // Once a marker, always a marker.
-    m_element = element;
-}
-
-bool HTMLFormattingElementList::Entry::operator==(const Entry& other) const
-{
-    return m_element == other.m_element;
-}
-
-bool HTMLFormattingElementList::Entry::operator!=(const Entry& other) const
-{
-    return m_element != other.m_element;
-}
-
 HTMLFormattingElementList::HTMLFormattingElementList()
 {
 }
index d5fcc0f..0c2a40f 100644 (file)
@@ -46,19 +46,33 @@ public:
     // access to Entry::isMarker() and Entry::replaceElement() to do so.
     class Entry {
     public:
-        Entry(Element*);
+        // Inline because they're hot and Vector<T> uses them.
+        Entry(Element* element)
+            : m_element(element)
+        {
+            ASSERT(element);
+        }
         enum MarkerEntryType { MarkerEntry };
-        Entry(MarkerEntryType);
-        ~Entry();
+        Entry(MarkerEntryType)
+            : m_element(0)
+        {
+        }
+        ~Entry() {}
 
-        bool isMarker() const;
+        bool isMarker() const { return !m_element; }
 
-        Element* element() const;
-        void replaceElement(PassRefPtr<Element>);
+        Element* element() const
+        {
+            // The fact that !m_element == isMarker() is an implementation detail
+            // callers should check isMarker() before calling element().
+            ASSERT(m_element);
+            return m_element.get();
+        }
+        void replaceElement(PassRefPtr<Element> element) { m_element = element; }
 
-        // Needed for use with Vector.
-        bool operator==(const Entry&) const;
-        bool operator!=(const Entry&) const;
+        // Needed for use with Vector.  These are super-hot and must be inline.
+        bool operator==(const Entry& other) const { return m_element == other.m_element; }
+        bool operator!=(const Entry& other) const { return m_element != other.m_element; }
 
     private:
         RefPtr<Element> m_element;