Change ActivationImp to be allocated via the garbage collector

	again instead of on the stack. This fixes the following four
	regressions but sadly it causes a 6% performance hit. It's
	probably possibly to reduce the hit a bit by being smarter about
	inlining and the way the marking list variant is implemented, but
	I'll look into that later.

	- fixed 3111500 - REGRESSION: crash in "KJS::ScopeChain::mark()" on www.posci.com
	- fixed 3111145 - REGRESSION: reproducible crash in KJS hashtable lookup at time.com
	- fixed 3110897 - REGRESSION: javascript crasher on http://bmwgallery.tripod.com/
	- fixed 3109987 - REGRESSION: Reproducible crash in KJS ObjectImp at live365.com

	Also:

	- improved DEBUG_COLLECTOR mode a bit by never giving memory back
	to the system.

        * kjs/collector.cpp:
        * kjs/context.h:
        * kjs/function.cpp:
        (ActivationImp::ActivationImp):
        (ActivationImp::mark):
        (ActivationImp::createArgumentsObject):
        * kjs/function.h:
        * kjs/internal.cpp:
        (ContextImp::ContextImp):
        (ContextImp::mark):
        * kjs/list.cpp:
        * kjs/list.h:
        * kjs/value.cpp:
        (Value::Value):

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 9a4ca3c2fb1c92a3572227593de93d3dcddf3652..d560640c3fd004374b05e146fb26c0693b193098 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,37 @@
+2002-11-26  Maciej Stachowiak  <mjs@apple.com>
+
+       Change ActivationImp to be allocated via the garbage collector
+       again instead of on the stack. This fixes the following four
+       regressions but sadly it causes a 6% performance hit. It's
+       probably possibly to reduce the hit a bit by being smarter about
+       inlining and the way the marking list variant is implemented, but
+       I'll look into that later.
+
+       - fixed 3111500 - REGRESSION: crash in "KJS::ScopeChain::mark()" on www.posci.com
+       - fixed 3111145 - REGRESSION: reproducible crash in KJS hashtable lookup at time.com
+       - fixed 3110897 - REGRESSION: javascript crasher on http://bmwgallery.tripod.com/
+       - fixed 3109987 - REGRESSION: Reproducible crash in KJS ObjectImp at live365.com
+       
+       Also:
+       
+       - improved DEBUG_COLLECTOR mode a bit by never giving memory back
+       to the system.
+       
+        * kjs/collector.cpp:
+        * kjs/context.h:
+        * kjs/function.cpp:
+        (ActivationImp::ActivationImp):
+        (ActivationImp::mark):
+        (ActivationImp::createArgumentsObject):
+        * kjs/function.h:
+        * kjs/internal.cpp:
+        (ContextImp::ContextImp):
+        (ContextImp::mark):
+        * kjs/list.cpp:
+        * kjs/list.h:
+        * kjs/value.cpp:
+        (Value::Value):
+
 2002-11-26  Darin Adler  <darin@apple.com>
 
         * kjs/property_map.cpp:

diff --git a/JavaScriptCore/ChangeLog-2002-12-03 b/JavaScriptCore/ChangeLog-2002-12-03
index 9a4ca3c2fb1c92a3572227593de93d3dcddf3652..d560640c3fd004374b05e146fb26c0693b193098 100644 (file)
--- a/JavaScriptCore/ChangeLog-2002-12-03
+++ b/JavaScriptCore/ChangeLog-2002-12-03
@@ -1,3 +1,37 @@
+2002-11-26  Maciej Stachowiak  <mjs@apple.com>
+
+       Change ActivationImp to be allocated via the garbage collector
+       again instead of on the stack. This fixes the following four
+       regressions but sadly it causes a 6% performance hit. It's
+       probably possibly to reduce the hit a bit by being smarter about
+       inlining and the way the marking list variant is implemented, but
+       I'll look into that later.
+
+       - fixed 3111500 - REGRESSION: crash in "KJS::ScopeChain::mark()" on www.posci.com
+       - fixed 3111145 - REGRESSION: reproducible crash in KJS hashtable lookup at time.com
+       - fixed 3110897 - REGRESSION: javascript crasher on http://bmwgallery.tripod.com/
+       - fixed 3109987 - REGRESSION: Reproducible crash in KJS ObjectImp at live365.com
+       
+       Also:
+       
+       - improved DEBUG_COLLECTOR mode a bit by never giving memory back
+       to the system.
+       
+        * kjs/collector.cpp:
+        * kjs/context.h:
+        * kjs/function.cpp:
+        (ActivationImp::ActivationImp):
+        (ActivationImp::mark):
+        (ActivationImp::createArgumentsObject):
+        * kjs/function.h:
+        * kjs/internal.cpp:
+        (ContextImp::ContextImp):
+        (ContextImp::mark):
+        * kjs/list.cpp:
+        * kjs/list.h:
+        * kjs/value.cpp:
+        (Value::Value):
+
 2002-11-26  Darin Adler  <darin@apple.com>
 
         * kjs/property_map.cpp:

diff --git a/JavaScriptCore/ChangeLog-2003-10-25 b/JavaScriptCore/ChangeLog-2003-10-25
index 9a4ca3c2fb1c92a3572227593de93d3dcddf3652..d560640c3fd004374b05e146fb26c0693b193098 100644 (file)
--- a/JavaScriptCore/ChangeLog-2003-10-25
+++ b/JavaScriptCore/ChangeLog-2003-10-25
@@ -1,3 +1,37 @@
+2002-11-26  Maciej Stachowiak  <mjs@apple.com>
+
+       Change ActivationImp to be allocated via the garbage collector
+       again instead of on the stack. This fixes the following four
+       regressions but sadly it causes a 6% performance hit. It's
+       probably possibly to reduce the hit a bit by being smarter about
+       inlining and the way the marking list variant is implemented, but
+       I'll look into that later.
+
+       - fixed 3111500 - REGRESSION: crash in "KJS::ScopeChain::mark()" on www.posci.com
+       - fixed 3111145 - REGRESSION: reproducible crash in KJS hashtable lookup at time.com
+       - fixed 3110897 - REGRESSION: javascript crasher on http://bmwgallery.tripod.com/
+       - fixed 3109987 - REGRESSION: Reproducible crash in KJS ObjectImp at live365.com
+       
+       Also:
+       
+       - improved DEBUG_COLLECTOR mode a bit by never giving memory back
+       to the system.
+       
+        * kjs/collector.cpp:
+        * kjs/context.h:
+        * kjs/function.cpp:
+        (ActivationImp::ActivationImp):
+        (ActivationImp::mark):
+        (ActivationImp::createArgumentsObject):
+        * kjs/function.h:
+        * kjs/internal.cpp:
+        (ContextImp::ContextImp):
+        (ContextImp::mark):
+        * kjs/list.cpp:
+        * kjs/list.h:
+        * kjs/value.cpp:
+        (Value::Value):
+
 2002-11-26  Darin Adler  <darin@apple.com>
 
         * kjs/property_map.cpp:

diff --git a/JavaScriptCore/kjs/collector.cpp b/JavaScriptCore/kjs/collector.cpp
index 8869ac057555a18a661542058cd4079165f93649..5d068230344a4c1254f4ba2ec32790557ddf06e2 100644 (file)
--- a/JavaScriptCore/kjs/collector.cpp
+++ b/JavaScriptCore/kjs/collector.cpp
@@ -162,6 +162,7 @@ void* Collector::allocate(size_t s)
 
 bool Collector::collect()
 {
+  puts("COLLECT");
   bool deleted = false;
 
   // MARK: first mark all referenced objects recursively
@@ -252,8 +253,9 @@ bool Collector::collect()
     if (heap.blocks[block]->usedCells == 0) {
       emptyBlocks++;
       if (emptyBlocks > SPARE_EMPTY_BLOCKS) {
-       delete heap.blocks[block];
-
+#if !DEBUG_COLLECTOR
+       free(heap.blocks[block]);
+#endif
        // swap with the last block so we compact as we go
        heap.blocks[block] = heap.blocks[heap.usedBlocks - 1];
        heap.usedBlocks--;
@@ -279,7 +281,11 @@ bool Collector::collect()
        imp->_flags == (ValueImp::VI_GCALLOWED | ValueImp::VI_CREATED)) {
       
       imp->~ValueImp();
+#if DEBUG_COLLECTOR
+      heap.oversizeCells[cell]->u.freeCell.zeroIfFree = 0;
+#else
       free((void *)imp);
+#endif
 
       // swap with the last oversize cell so we compact as we go
       heap.oversizeCells[cell] = heap.oversizeCells[heap.usedOversizeCells - 1];

diff --git a/JavaScriptCore/kjs/context.h b/JavaScriptCore/kjs/context.h
index d931d7a4cca6c3da4a2c661f29a3714df6ec676d..5579944377408cebf40609ae2c80c0ff4d126488 100644 (file)
--- a/JavaScriptCore/kjs/context.h
+++ b/JavaScriptCore/kjs/context.h
@@ -55,7 +55,6 @@ namespace KJS  {
   private:
     InterpreterImp *_interpreter;
     ContextImp *_callingContext;
-    ActivationImp _activationImp;
     FunctionImp *_function;
     const List *_arguments;
     Object activation;

diff --git a/JavaScriptCore/kjs/function.cpp b/JavaScriptCore/kjs/function.cpp
index b95ba8a30a6e5fa910d8279c217e861acaa4ec01..16b505654cb5f74e21b9d962fd49a45833f43bb0 100644 (file)
--- a/JavaScriptCore/kjs/function.cpp
+++ b/JavaScriptCore/kjs/function.cpp
@@ -332,9 +332,10 @@ ArgumentsImp::ArgumentsImp(ExecState *exec, FunctionImp *func, const List &args)
 const ClassInfo ActivationImp::info = {"Activation", 0, 0, 0};
 
 // ECMA 10.1.6
-ActivationImp::ActivationImp(ContextImp *context)
-    : _context(context), _argumentsObject(0)
+ActivationImp::ActivationImp(FunctionImp *function, const List &arguments)
+    : _function(function), _arguments(true), _argumentsObject(0)
 {
+  _arguments = arguments.copy();
   // FIXME: Do we need to support enumerating the arguments property?
 }
 
@@ -373,6 +374,9 @@ bool ActivationImp::deleteProperty(ExecState *exec, const Identifier &propertyNa
 
 void ActivationImp::mark()
 {
+    if (_function && !_function->marked()) 
+        _function->mark();
+    _arguments.mark();
     if (_argumentsObject && !_argumentsObject->marked())
         _argumentsObject->mark();
     ObjectImp::mark();
@@ -380,12 +384,7 @@ void ActivationImp::mark()
 
 void ActivationImp::createArgumentsObject(ExecState *exec) const
 {
-    FunctionImp *function = _context->function();
-    const List *arguments = _context->arguments();
-    if (arguments)
-        _argumentsObject = new ArgumentsImp(exec, function, *arguments);
-    else
-        _argumentsObject = new ArgumentsImp(exec, function);
+  _argumentsObject = new ArgumentsImp(exec, _function, _arguments);
 }
 
 // ------------------------------ GlobalFunc -----------------------------------

diff --git a/JavaScriptCore/kjs/function.h b/JavaScriptCore/kjs/function.h
index 42f7d1f0a365de1b2020ed1f056e77213bfd30cb..bad1711590bf542c837d2490faba5afd69959587 100644 (file)
--- a/JavaScriptCore/kjs/function.h
+++ b/JavaScriptCore/kjs/function.h
@@ -100,7 +100,7 @@ namespace KJS {
 
   class ActivationImp : public ObjectImp {
   public:
-    ActivationImp(ContextImp *);
+    ActivationImp(FunctionImp *function, const List &arguments);
 
     virtual Value get(ExecState *exec, const Identifier &propertyName) const;
     virtual void put(ExecState *exec, const Identifier &propertyName, const Value &value, int attr = None);
@@ -115,7 +115,8 @@ namespace KJS {
   private:
     void createArgumentsObject(ExecState *exec) const;
     
-    const ContextImp *_context;
+    FunctionImp *_function;
+    List _arguments;
     mutable ArgumentsImp *_argumentsObject;
   };
 

diff --git a/JavaScriptCore/kjs/internal.cpp b/JavaScriptCore/kjs/internal.cpp
index 3a3b55eabb1292ced58524afef649c3ea1d34026..0d33dd9e1db71f5aea6b9515b99cf081d144c97f 100644 (file)
--- a/JavaScriptCore/kjs/internal.cpp
+++ b/JavaScriptCore/kjs/internal.cpp
@@ -359,14 +359,14 @@ void LabelStack::clear()
 // ECMA 10.2
 ContextImp::ContextImp(Object &glob, InterpreterImp *interpreter, Object &thisV, CodeType type,
                        ContextImp *callingCon, FunctionImp *func, const List *args)
-    : _interpreter(interpreter), _activationImp(this), _function(func), _arguments(args)
+    : _interpreter(interpreter), _function(func), _arguments(args)
 {
   codeType = type;
   _callingContext = callingCon;
 
   // create and initialize activation object (ECMA 10.1.6)
   if (type == FunctionCode || type == AnonymousCode ) {
-    activation = Object(&_activationImp);
+    activation = Object(new ActivationImp(func, *args));
     variable = activation;
   } else {
     activation = Object();
@@ -414,10 +414,6 @@ void ContextImp::mark()
 {
   for (ContextImp *context = this; context; context = context->_callingContext) {
     context->scope.mark();
-    context->_activationImp.mark();
-#if DEBUG_COLLECTOR
-    context->_activationImp._flags &= ~ValueImp::VI_MARKED;
-#endif
   }
 }
 

diff --git a/JavaScriptCore/kjs/list.cpp b/JavaScriptCore/kjs/list.cpp
index a06c6c27fd77d1ae20cd3d4febf55d4ed2db6153..fc281bbcf2d47792ded7752b93606c1b756ea23e 100644 (file)
--- a/JavaScriptCore/kjs/list.cpp
+++ b/JavaScriptCore/kjs/list.cpp
@@ -28,7 +28,7 @@
 namespace KJS {
 
 // tunable parameters
-const int poolSize = 16; // must be a power of 2
+const int poolSize = 32; // must be a power of 2
 const int inlineValuesSize = 4;
 
 // derived constants
@@ -114,7 +114,7 @@ static inline void deallocateListImp(ListImp *imp)
         delete imp;
 }
 
-List::List() : _impBase(allocateListImp())
+List::List() : _impBase(allocateListImp()), _needsMarking(false)
 {
     ListImp *imp = static_cast<ListImp *>(_impBase);
     imp->size = 0;
@@ -122,6 +122,28 @@ List::List() : _impBase(allocateListImp())
     imp->capacity = 0;
     imp->overflow = 0;
 
+    if (!_needsMarking) {
+       imp->valueRefCount = 1;
+    }
+#if DUMP_STATISTICS
+    if (++numLists > numListsHighWaterMark)
+        numListsHighWaterMark = numLists;
+    imp->sizeHighWaterMark = 0;
+#endif
+}
+
+List::List(bool needsMarking) : _impBase(allocateListImp()), _needsMarking(needsMarking)
+{
+    ListImp *imp = static_cast<ListImp *>(_impBase);
+    imp->size = 0;
+    imp->refCount = 1;
+    imp->capacity = 0;
+    imp->overflow = 0;
+
+    if (!_needsMarking) {
+       imp->valueRefCount = 1;
+    }
+
 #if DUMP_STATISTICS
     if (++numLists > numListsHighWaterMark)
         numListsHighWaterMark = numLists;
@@ -129,7 +151,7 @@ List::List() : _impBase(allocateListImp())
 #endif
 }
 
-inline void List::derefValues()
+void List::derefValues()
 {
     ListImp *imp = static_cast<ListImp *>(_impBase);
     
@@ -145,6 +167,44 @@ inline void List::derefValues()
         overflow[i]->deref();
 }
 
+void List::refValues()
+{
+    ListImp *imp = static_cast<ListImp *>(_impBase);
+    
+    int size = imp->size;
+    
+    int inlineSize = MIN(size, inlineValuesSize);
+    for (int i = 0; i != inlineSize; ++i)
+        imp->values[i]->ref();
+    
+    int overflowSize = size - inlineSize;
+    ValueImp **overflow = imp->overflow;
+    for (int i = 0; i != overflowSize; ++i)
+        overflow[i]->ref();
+}
+
+void List::markValues()
+{
+    ListImp *imp = static_cast<ListImp *>(_impBase);
+    
+    int size = imp->size;
+    
+    int inlineSize = MIN(size, inlineValuesSize);
+    for (int i = 0; i != inlineSize; ++i) {
+       if (!imp->values[i]->marked()) {
+           imp->values[i]->mark();
+       }
+    }
+
+    int overflowSize = size - inlineSize;
+    ValueImp **overflow = imp->overflow;
+    for (int i = 0; i != overflowSize; ++i) {
+       if (!overflow[i]->marked()) {
+           overflow[i]->mark();
+       }
+    }
+}
+
 void List::release()
 {
     ListImp *imp = static_cast<ListImp *>(_impBase);
@@ -157,7 +217,6 @@ void List::release()
             ++numListsBiggerThan[i];
 #endif
 
-    derefValues();
     delete [] imp->overflow;
     deallocateListImp(imp);
 }
@@ -174,7 +233,9 @@ ValueImp *List::impAt(int i) const
 
 void List::clear()
 {
-    derefValues();
+    if (_impBase->valueRefCount > 0) {
+       derefValues();
+    }
     _impBase->size = 0;
 }
 
@@ -189,7 +250,9 @@ void List::append(ValueImp *v)
         listSizeHighWaterMark = imp->size;
 #endif
 
-    v->ref();
+    if (imp->valueRefCount > 0) {
+       v->ref();
+    }
     
     if (i < inlineValuesSize) {
         imp->values[i] = v;
@@ -211,7 +274,7 @@ void List::append(ValueImp *v)
     imp->overflow[i - inlineValuesSize] = v;
 }
 
-List List::copyTail() const
+List List::copy() const
 {
     List copy;
 
@@ -231,6 +294,27 @@ List List::copyTail() const
     return copy;
 }
 
+
+List List::copyTail() const
+{
+    List copy;
+
+    ListImp *imp = static_cast<ListImp *>(_impBase);
+
+    int size = imp->size;
+
+    int inlineSize = MIN(size, inlineValuesSize);
+    for (int i = 1; i != inlineSize; ++i)
+        copy.append(imp->values[i]);
+
+    ValueImp **overflow = imp->overflow;
+    int overflowSize = size - inlineSize;
+    for (int i = 0; i != overflowSize; ++i)
+        copy.append(overflow[i]);
+
+    return copy;
+}
+
 const List &List::empty()
 {
     static List emptyList;

diff --git a/JavaScriptCore/kjs/list.h b/JavaScriptCore/kjs/list.h
index 73761d78b3bfa95b2c2f5e7489337c2dde9d6b4f..d625b3c4d72bc1dbae55aadb640ccff21688236a 100644 (file)
--- a/JavaScriptCore/kjs/list.h
+++ b/JavaScriptCore/kjs/list.h
@@ -30,6 +30,7 @@ namespace KJS {
     struct ListImpBase {
         int size;
         int refCount;
+       int valueRefCount;
     };
     
     class ListIterator;
@@ -47,9 +48,14 @@ namespace KJS {
     class List {
     public:
         List();
+       List(bool needsMarking);
         ~List() { deref(); }
 
-        List(const List &b) : _impBase(b._impBase) { ++_impBase->refCount; }
+        List(const List &b) : _impBase(b._impBase), _needsMarking(false) {
+           ++_impBase->refCount; 
+           if (!_impBase->valueRefCount) refValues(); 
+           ++_impBase->valueRefCount; 
+       }
         List &operator=(const List &);
 
         /**
@@ -63,6 +69,12 @@ namespace KJS {
          * Remove all elements from the list.
          */
         void clear();
+
+        /**
+         * Make a copy of the list
+         */
+        List copy() const;
+
         /**
          * Make a copy of the list, omitting the first element.
          */
@@ -107,13 +119,17 @@ namespace KJS {
          */
         static const List &empty();
         
+       void mark() { if (_impBase->valueRefCount == 0) markValues(); }
     private:
         ListImpBase *_impBase;
+       bool _needsMarking;
         
-        void deref() { if (--_impBase->refCount == 0) release(); }
+        void deref() { if (!_needsMarking && --_impBase->valueRefCount == 0) derefValues(); if (--_impBase->refCount == 0) release(); }
 
         void release();
+        void refValues();
         void derefValues();
+        void markValues();
     };
   
     /**
@@ -176,6 +192,13 @@ namespace KJS {
         ++bImpBase->refCount;
         deref();
         _impBase = bImpBase;
+       if (!_needsMarking) {
+           if (!_impBase->valueRefCount) {
+               refValues();
+           }
+           _impBase->valueRefCount++;
+       }
+
         return *this;
     }
 

diff --git a/JavaScriptCore/kjs/value.cpp b/JavaScriptCore/kjs/value.cpp
index e701c49bc8e9e9ee17e1fff3749dcda06614ea59..5d19c238277b125030a958fa95b94a30860973f2 100644 (file)
--- a/JavaScriptCore/kjs/value.cpp
+++ b/JavaScriptCore/kjs/value.cpp
@@ -203,7 +203,7 @@ Value::Value(ValueImp *v)
   rep = v;
 #if DEBUG_COLLECTOR
   assert (!(rep && !SimpleNumber::is(rep) && *((uint32_t *)rep) == 0 ));
-  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & VI_MARKED));
+  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & ValueImp::VI_MARKED));
 #endif
   if (v)
   {
@@ -218,7 +218,7 @@ Value::Value(const Value &v)
   rep = v.imp();
 #if DEBUG_COLLECTOR
   assert (!(rep && !SimpleNumber::is(rep) && *((uint32_t *)rep) == 0 ));
-  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & VI_MARKED));
+  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & ValueImp::VI_MARKED));
 #endif
   if (rep)
   {