Reviewed by Darin.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Apr 2007 04:16:42 +0000 (04:16 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Apr 2007 04:16:42 +0000 (04:16 +0000)
        - discard the arguments List for an ActivationImp when the corresponding Context is destroyed (1.7% speedup)
        http://bugs.webkit.org/show_bug.cgi?id=13385

        Based an idea by Christopher E. Hyde <C.Hyde@parableuk.force9.co.uk>. His patch to do
        this also had many other List changes and I found this much simpler subset of the changes
        was actually a hair faster.

        This optimization is valid because the arguments list is only kept around to
        lazily make the arguments object. If it's not made by the time the function
        exits, it never will be, since any function that captures the continuation will
        have its own local arguments variable in scope.

        Besides the 1.7% speed improvement, it shrinks List by 4 bytes
        (which in turn shrinks ActivationImp by 4 bytes).

        * kjs/Context.cpp:
        (KJS::Context::~Context): Clear the activation's arguments list.
        * kjs/function.cpp:
        (KJS::ActivationImp::ActivationImp): Adjusted for list changes.
        (KJS::ActivationImp::mark): No need to mark, lists are always protected (this doesn't
        cause a ref-cycle for reasons stated above).
        (KJS::ActivationImp::createArgumentsObject): Clear arguments list.
        * kjs/function.h:
        * kjs/list.cpp:
        (KJS::List::List): No more needsMarking boolean
        (KJS::List::operator=): ditto
        * kjs/list.h:
        (KJS::List::List): ditto
        (KJS::List::reset): ditto
        (KJS::List::deref): ditto

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/Context.cpp
JavaScriptCore/kjs/function.cpp
JavaScriptCore/kjs/function.h
JavaScriptCore/kjs/list.cpp
JavaScriptCore/kjs/list.h

index c597ce4e12a7efb27c34cbfedc503e707e3cf066..54c3fa606c9957aa32d0a9bf77a06b9cab8f0e06 100644 (file)
@@ -1,3 +1,38 @@
+2007-04-22  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+        
+        - discard the arguments List for an ActivationImp when the corresponding Context is destroyed (1.7% speedup)
+        http://bugs.webkit.org/show_bug.cgi?id=13385
+
+        Based an idea by Christopher E. Hyde <C.Hyde@parableuk.force9.co.uk>. His patch to do 
+        this also had many other List changes and I found this much simpler subset of the changes
+        was actually a hair faster.
+        
+        This optimization is valid because the arguments list is only kept around to
+        lazily make the arguments object. If it's not made by the time the function
+        exits, it never will be, since any function that captures the continuation will
+        have its own local arguments variable in scope.
+        
+        Besides the 1.7% speed improvement, it shrinks List by 4 bytes
+        (which in turn shrinks ActivationImp by 4 bytes).
+        
+        * kjs/Context.cpp:
+        (KJS::Context::~Context): Clear the activation's arguments list.
+        * kjs/function.cpp:
+        (KJS::ActivationImp::ActivationImp): Adjusted for list changes.
+        (KJS::ActivationImp::mark): No need to mark, lists are always protected (this doesn't
+        cause a ref-cycle for reasons stated above).
+        (KJS::ActivationImp::createArgumentsObject): Clear arguments list.
+        * kjs/function.h:
+        * kjs/list.cpp:
+        (KJS::List::List): No more needsMarking boolean
+        (KJS::List::operator=): ditto
+        * kjs/list.h:
+        (KJS::List::List): ditto
+        (KJS::List::reset): ditto
+        (KJS::List::deref): ditto
+
 2007-04-22  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin.
index 90467c3b93deb1ac160c733cd94ec9138a4f71f4..caf1ff824e6853f775d7cfb624e54a5f3e5efce0 100644 (file)
@@ -1,9 +1,9 @@
-// -*- c-basic-offset: 2 -*-
+// -*- mode: c++; c-basic-offset: 4 -*-
 /*
  *  This file is part of the KDE libraries
  *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
  *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
- *  Copyright (C) 2003, 2006 Apple Computer, Inc.
+ *  Copyright (C) 2003, 2006-2007 Apple Computer, Inc.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
@@ -84,6 +84,14 @@ Context::Context(JSObject* glob, Interpreter* interpreter, JSObject* thisV,
 Context::~Context()
 {
     m_interpreter->setContext(m_callingContext);
+
+    // The arguments list is only needed to potentially create the  arguments object, 
+    // which isn't accessible from nested scopes so we can discard the list as soon 
+    // as the function is done running.
+    // This prevents lists of Lists from building up, waiting to be garbage collected
+    ActivationImp* activation = static_cast<ActivationImp*>(m_activation);
+    if (activation)
+        activation->releaseArguments();
 }
 
 void Context::mark()
index 68345f828f40fd83055c3f030f5091f1d41c0245..f797713e5c2326247124078c6a91a6e3e9baf10a 100644 (file)
@@ -509,9 +509,8 @@ const ClassInfo ActivationImp::info = {"Activation", 0, 0, 0};
 
 // ECMA 10.1.6
 ActivationImp::ActivationImp(FunctionImp* function, const List& arguments)
-    : _function(function), _arguments(true), _argumentsObject(0)
+    : _function(function), _arguments(arguments), _argumentsObject(0)
 {
-  _arguments.copyFrom(arguments);
   // FIXME: Do we need to support enumerating the arguments property?
 }
 
@@ -570,15 +569,16 @@ void ActivationImp::mark()
 {
     if (_function && !_function->marked()) 
         _function->mark();
-    _arguments.mark();
     if (_argumentsObject && !_argumentsObject->marked())
         _argumentsObject->mark();
     JSObject::mark();
 }
 
-void ActivationImp::createArgumentsObject(ExecState* exec) const
+void ActivationImp::createArgumentsObject(ExecState* exec)
 {
   _argumentsObject = new Arguments(exec, _function, _arguments, const_cast<ActivationImp*>(this));
+  // The arguments list is only needed to create the arguments object, so discard it now
+  _arguments.reset();
 }
 
 // ------------------------------ GlobalFunc -----------------------------------
index 3ec7ab0956d057ae998cfb7498128d3d3bc87ad0..bb37283ed70ea901a4b7fb1172812862527e7a6c 100644 (file)
@@ -170,10 +170,13 @@ namespace KJS {
     virtual void mark();
 
     bool isActivation() { return true; }
+
+    void releaseArguments() { _arguments.reset(); }
+
   private:
     static PropertySlot::GetValueFunc getArgumentsGetter();
     static JSValue* argumentsGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot& slot);
-    void createArgumentsObject(ExecState*) const;
+    void createArgumentsObject(ExecState*);
     
     FunctionImp* _function;
     List _arguments;
index bab0a7ad5e29972355040fa48776e56cf202bc8f..5a3f0c2508e51227a17594b962bae5c96490a809 100644 (file)
@@ -164,7 +164,7 @@ static inline ListImp *allocateListImp()
     return imp;
 }
 
-List::List() : _impBase(allocateListImp()), _needsMarking(false)
+List::List() : _impBase(allocateListImp())
 {
     ListImp *imp = static_cast<ListImp *>(_impBase);
     imp->size = 0;
@@ -180,22 +180,6 @@ List::List() : _impBase(allocateListImp()), _needsMarking(false)
 #endif
 }
 
-List::List(bool needsMarking) : _impBase(allocateListImp()), _needsMarking(needsMarking)
-{
-    ListImp *imp = static_cast<ListImp *>(_impBase);
-    imp->size = 0;
-    imp->refCount = 1;
-    imp->valueRefCount = !needsMarking;
-    imp->capacity = 0;
-    imp->overflow = 0;
-
-#if DUMP_STATISTICS
-    if (++numLists > numListsHighWaterMark)
-        numListsHighWaterMark = numLists;
-    imp->sizeHighWaterMark = 0;
-#endif
-}
-
 void List::markValues()
 {
     static_cast<ListImp *>(_impBase)->markValues();
@@ -346,8 +330,7 @@ List &List::operator=(const List &b)
 {
     ListImpBase *bImpBase = b._impBase;
     ++bImpBase->refCount;
-    if (!_needsMarking)
-        ++bImpBase->valueRefCount;
+    ++bImpBase->valueRefCount;
     deref();
     _impBase = bImpBase;
     return *this;
index e0e4e5a4ee983cf681fc23cc902007a23e91c1ca..194a464f00f2f655e8d3dae55412f4c8549599f1 100644 (file)
@@ -48,10 +48,9 @@ namespace KJS {
     class List {
     public:
         List();
-        explicit List(bool needsMarking);
         ~List() { deref(); }
 
-        List(const List &b) : _impBase(b._impBase), _needsMarking(false) {
+        List(const List &b) : _impBase(b._impBase) {
             ++_impBase->refCount; 
             ++_impBase->valueRefCount; 
         }
@@ -68,6 +67,8 @@ namespace KJS {
          */
         void clear();
 
+        void reset() { deref(); ++(_impBase = empty()._impBase)->refCount; }
+
         /**
          * Make a copy of the list
          */
@@ -120,14 +121,11 @@ namespace KJS {
          */
         static const List &empty();
         
-        void mark() { if (_impBase->valueRefCount == 0) markValues(); }
-
         static void markProtectedLists();
     private:
         ListImpBase *_impBase;
-        bool _needsMarking;
         
-        void deref() { if (!_needsMarking) --_impBase->valueRefCount; if (--_impBase->refCount == 0) release(); }
+        void deref() { --_impBase->valueRefCount; if (--_impBase->refCount == 0) release(); }
 
         void release();
         void markValues();