Reviewed by Geoff.
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Jan 2008 19:13:40 +0000 (19:13 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Jan 2008 19:13:40 +0000 (19:13 +0000)
        - http://bugs.webkit.org/show_bug.cgi?id=16683
          speed up function calls by making ScopeChain::push cheaper

        This gives a 1.019x speedup on SunSpider.

        After doing this, I realized this probably will be obsolete when the optimization
        to avoid creating an activation object is done. When we do that one we should check
        if rolling this out will speed things up, since this does add overhead at the time
        you copy the scope chain.

        * kjs/object.h: Removed the ScopeChain::release function. It was
        marked inline, and called in exactly one place, so moved it there.
        No idea why it was in this header file!

        * kjs/scope_chain.cpp: Removed the overload of the ScopeChain::push
        function that takes another ScopeChain. It was unused. I think we used
        it over in WebCore at one point, but not any more.

        * kjs/scope_chain.h: Changed ScopeChainNode into a struct rather than
        a class, got rid of its constructor so we can have one that's uninitialized,
        and moved the refCount into a derived struct, ScopeChainHeapNode. Made _node
        mutable so it can be changed in the moveToHeap function. Changed the copy
        constructor and assignment operator to call moveToHeap, since the top node
        can't be shared when it's embedded in another ScopeChain object. Updated
        functions as needed to handle the case where the first object isn't on the
        heap or to add casts for cases where it's guaranteed to be. Changed the push
        function to always put the new node into the ScopeChain object; it will get
        put onto the heap when needed later.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/object.h
JavaScriptCore/kjs/scope_chain.cpp
JavaScriptCore/kjs/scope_chain.h

index 492177357f958e2fd18979323fe4a371482c284d..b813b719c896ddf54ef0938281d45b3bb22ab4c9 100644 (file)
@@ -1,3 +1,36 @@
+2008-01-01  Darin Adler  <darin@apple.com>
+
+        Reviewed by Geoff.
+
+        - http://bugs.webkit.org/show_bug.cgi?id=16683
+          speed up function calls by making ScopeChain::push cheaper
+
+        This gives a 1.019x speedup on SunSpider.
+
+        After doing this, I realized this probably will be obsolete when the optimization
+        to avoid creating an activation object is done. When we do that one we should check
+        if rolling this out will speed things up, since this does add overhead at the time
+        you copy the scope chain.
+
+        * kjs/object.h: Removed the ScopeChain::release function. It was
+        marked inline, and called in exactly one place, so moved it there.
+        No idea why it was in this header file!
+
+        * kjs/scope_chain.cpp: Removed the overload of the ScopeChain::push
+        function that takes another ScopeChain. It was unused. I think we used
+        it over in WebCore at one point, but not any more.
+
+        * kjs/scope_chain.h: Changed ScopeChainNode into a struct rather than
+        a class, got rid of its constructor so we can have one that's uninitialized,
+        and moved the refCount into a derived struct, ScopeChainHeapNode. Made _node
+        mutable so it can be changed in the moveToHeap function. Changed the copy
+        constructor and assignment operator to call moveToHeap, since the top node
+        can't be shared when it's embedded in another ScopeChain object. Updated
+        functions as needed to handle the case where the first object isn't on the
+        heap or to add casts for cases where it's guaranteed to be. Changed the push
+        function to always put the new node into the ScopeChain object; it will get
+        put onto the heap when needed later.
+
 2008-01-01  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Darin Adler.
index caa5a861d85f904f6c5745e22b3caf101129e10c..28643bafd7977349dab675e7a3db6c9ca3033fd8 100644 (file)
@@ -592,19 +592,6 @@ inline void ScopeChain::mark()
     }
 }
 
-inline void ScopeChain::release()
-{
-    // This function is only called by deref(),
-    // Deref ensures these conditions are true.
-    ASSERT(_node && _node->refCount == 0);
-    ScopeChainNode *n = _node;
-    do {
-        ScopeChainNode *next = n->next;
-        delete n;
-        n = next;
-    } while (n && --n->refCount == 0);
-}
-
 inline JSValue* JSObject::toPrimitive(ExecState* exec, JSType preferredType) const
 {
     return defaultValue(exec, preferredType);
index aba066ad1c068987531174f2262ee4b2808c68bb..96f21ca80c9db43dd1ee1f56e658ddc8611eccf6 100644 (file)
 
 namespace KJS {
 
-void ScopeChain::push(const ScopeChain &c)
-{
-    ScopeChainNode **tail = &_node;
-    for (ScopeChainNode *n = c._node; n; n = n->next) {
-        ScopeChainNode *newNode = new ScopeChainNode(*tail, n->object);
-        *tail = newNode;
-        tail = &newNode->next;
-    }
-}
-
 #ifndef NDEBUG
 
 void ScopeChain::print()
index a10abcdbba00eb2d903ff6113027ac3e514499ce..987bff81f93e93abcbffa965fde59e7502767b2a 100644 (file)
 namespace KJS {
 
     class JSObject;
-    class ExecState;
+    struct ScopeChainHeapNode;
     
-    class ScopeChainNode {
-    public:
-        ScopeChainNode(ScopeChainNode *n, JSObject *o)
-            : next(n), object(o), refCount(1) { }
+    struct ScopeChainNode {
+        JSObject* object;
+        ScopeChainHeapNode* next;
+    };
 
-        ScopeChainNode *next;
-        JSObject *object;
+    struct ScopeChainHeapNode : ScopeChainNode {
         int refCount;
     };
 
@@ -65,8 +64,7 @@ namespace KJS {
         ScopeChain() : _node(0) { }
         ~ScopeChain() { deref(); }
 
-        ScopeChain(const ScopeChain &c) : _node(c._node)
-            { if (_node) ++_node->refCount; }
+        ScopeChain(const ScopeChain&);
         ScopeChain &operator=(const ScopeChain &);
 
         bool isEmpty() const { return !_node; }
@@ -79,7 +77,6 @@ namespace KJS {
 
         void clear() { deref(); _node = 0; }
         void push(JSObject *);
-        void push(const ScopeChain &);
         void pop();
         
         void mark();
@@ -89,27 +86,65 @@ namespace KJS {
 #endif
         
     private:
-        ScopeChainNode *_node;
-        
-        void deref() { if (_node && --_node->refCount == 0) release(); }
+        mutable ScopeChainNode* _node;
+        ScopeChainNode m_initialTopNode;
+
+        ScopeChainHeapNode* moveToHeap() const;
+
+        void deref();
         void ref() const;
-        
-        void release();
     };
 
+inline ScopeChainHeapNode* ScopeChain::moveToHeap() const
+{
+    if (_node != &m_initialTopNode)
+        return static_cast<ScopeChainHeapNode*>(_node);
+    ScopeChainHeapNode* heapNode = new ScopeChainHeapNode;
+    heapNode->object = m_initialTopNode.object;
+    heapNode->next = m_initialTopNode.next;
+    heapNode->refCount = 1;
+    _node = heapNode;
+    return heapNode;
+}
+
+inline ScopeChain::ScopeChain(const ScopeChain& otherChain)
+{
+    if (!otherChain._node)
+        _node = 0;
+    else {
+        ScopeChainHeapNode* top = otherChain.moveToHeap();
+        ++top->refCount;
+        _node = top;
+    }
+}
+
 inline void ScopeChain::ref() const
 {
-    for (ScopeChainNode *n = _node; n; n = n->next) {
-        if (n->refCount++ != 0)
+    ASSERT(_node != &m_initialTopNode);
+    for (ScopeChainHeapNode* n = static_cast<ScopeChainHeapNode*>(_node); n; n = n->next) {
+        if (n->refCount++)
             break;
     }
 }
 
-inline ScopeChain &ScopeChain::operator=(const ScopeChain &c)
+inline void ScopeChain::deref()
 {
-    c.ref();
+    ScopeChainHeapNode* node = static_cast<ScopeChainHeapNode*>(_node);
+    if (node == &m_initialTopNode)
+        node = node->next;
+    ScopeChainHeapNode* next;
+    for (; node && --node->refCount == 0; node = next) {
+        next = node->next;
+        delete node;
+    }
+}
+
+inline ScopeChain &ScopeChain::operator=(const ScopeChain& otherChain)
+{
+    otherChain.moveToHeap();
+    otherChain.ref();
     deref();
-    _node = c._node;
+    _node = otherChain._node;
     return *this;
 }
 
@@ -123,24 +158,30 @@ inline JSObject *ScopeChain::bottom() const
     return last->object;
 }
 
-inline void ScopeChain::push(JSObject *o)
+inline void ScopeChain::push(JSObjecto)
 {
     ASSERT(o);
-    _node = new ScopeChainNode(_node, o);
+    ScopeChainHeapNode* heapNode = moveToHeap();
+    m_initialTopNode.object = o;
+    m_initialTopNode.next = heapNode;
+    _node = &m_initialTopNode;
 }
 
 inline void ScopeChain::pop()
 {
     ScopeChainNode *oldNode = _node;
     ASSERT(oldNode);
-    ScopeChainNode *newNode = oldNode->next;
+    ScopeChainHeapNode *newNode = oldNode->next;
     _node = newNode;
-    
-    if (--oldNode->refCount != 0) {
-        if (newNode)
-            ++newNode->refCount;
-    } else {
-        delete oldNode;
+
+    if (oldNode != &m_initialTopNode) {
+        ScopeChainHeapNode* oldHeapNode = static_cast<ScopeChainHeapNode*>(oldNode);
+        if (--oldHeapNode->refCount != 0) {
+            if (newNode)
+                ++newNode->refCount;
+        } else {
+            delete oldHeapNode;
+        }
     }
 }