Changeset 2883 in webkit for trunk/JavaScriptCore/kjs


Ignore:
Timestamp:
Nov 26, 2002, 3:52:00 PM (23 years ago)
Author:
mjs
Message:

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):
Location:
trunk/JavaScriptCore/kjs
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/kjs/collector.cpp

    r2845 r2883  
    163163bool Collector::collect()
    164164{
     165  puts("COLLECT");
    165166  bool deleted = false;
    166167
    … …  
    253254      emptyBlocks++;
    254255      if (emptyBlocks > SPARE_EMPTY_BLOCKS) {
    255         delete heap.blocks[block];
    256 
     256#if !DEBUG_COLLECTOR
     257        free(heap.blocks[block]);
     258#endif
    257259        // swap with the last block so we compact as we go
    258260        heap.blocks[block] = heap.blocks[heap.usedBlocks - 1];
    … …  
    280282     
    281283      imp->~ValueImp();
     284#if DEBUG_COLLECTOR
     285      heap.oversizeCells[cell]->u.freeCell.zeroIfFree = 0;
     286#else
    282287      free((void *)imp);
     288#endif
    283289
    284290      // swap with the last oversize cell so we compact as we go

  • trunk/JavaScriptCore/kjs/context.h

    r2851 r2883  
    5656    InterpreterImp *_interpreter;
    5757    ContextImp *_callingContext;
    58     ActivationImp _activationImp;
    5958    FunctionImp *_function;
    6059    const List *_arguments;

  • trunk/JavaScriptCore/kjs/function.cpp

    r2824 r2883  
    333333
    334334// ECMA 10.1.6
    335 ActivationImp::ActivationImp(ContextImp *context)
    336     : _context(context), _argumentsObject(0)
    337 {
     335ActivationImp::ActivationImp(FunctionImp *function, const List &arguments)
     336    : _function(function), _arguments(true), _argumentsObject(0)
     337{
     338  _arguments = arguments.copy();
    338339  // FIXME: Do we need to support enumerating the arguments property?
    339340}
    … …  
    374375void ActivationImp::mark()
    375376{
     377    if (_function && !_function->marked())
     378        _function->mark();
     379    _arguments.mark();
    376380    if (_argumentsObject && !_argumentsObject->marked())
    377381        _argumentsObject->mark();
    … …  
    381385void ActivationImp::createArgumentsObject(ExecState *exec) const
    382386{
    383     FunctionImp *function = _context->function();
    384     const List *arguments = _context->arguments();
    385     if (arguments)
    386         _argumentsObject = new ArgumentsImp(exec, function, *arguments);
    387     else
    388         _argumentsObject = new ArgumentsImp(exec, function);
     387  _argumentsObject = new ArgumentsImp(exec, _function, _arguments);
    389388}
    390389

  • trunk/JavaScriptCore/kjs/function.h

    r2821 r2883  
    101101  class ActivationImp : public ObjectImp {
    102102  public:
    103     ActivationImp(ContextImp *);
     103    ActivationImp(FunctionImp *function, const List &arguments);
    104104
    105105    virtual Value get(ExecState *exec, const Identifier &propertyName) const;
    … …  
    116116    void createArgumentsObject(ExecState *exec) const;
    117117   
    118     const ContextImp *_context;
     118    FunctionImp *_function;
     119    List _arguments;
    119120    mutable ArgumentsImp *_argumentsObject;
    120121  };

  • trunk/JavaScriptCore/kjs/internal.cpp

    r2851 r2883  
    360360ContextImp::ContextImp(Object &glob, InterpreterImp *interpreter, Object &thisV, CodeType type,
    361361                       ContextImp *callingCon, FunctionImp *func, const List *args)
    362     : _interpreter(interpreter), _activationImp(this), _function(func), _arguments(args)
     362    : _interpreter(interpreter), _function(func), _arguments(args)
    363363{
    364364  codeType = type;
    … …  
    367367  // create and initialize activation object (ECMA 10.1.6)
    368368  if (type == FunctionCode || type == AnonymousCode ) {
    369     activation = Object(&_activationImp);
     369    activation = Object(new ActivationImp(func, *args));
    370370    variable = activation;
    371371  } else {
    … …  
    415415  for (ContextImp *context = this; context; context = context->_callingContext) {
    416416    context->scope.mark();
    417     context->_activationImp.mark();
    418 #if DEBUG_COLLECTOR
    419     context->_activationImp._flags &= ~ValueImp::VI_MARKED;
    420 #endif
    421417  }
    422418}

  • trunk/JavaScriptCore/kjs/list.cpp

    r2843 r2883  
    2929
    3030// tunable parameters
    31 const int poolSize = 16; // must be a power of 2
     31const int poolSize = 32; // must be a power of 2
    3232const int inlineValuesSize = 4;
    3333
    … …  
    115115}
    116116
    117 List::List() : _impBase(allocateListImp())
     117List::List() : _impBase(allocateListImp()), _needsMarking(false)
    118118{
    119119    ListImp *imp = static_cast<ListImp *>(_impBase);
    … …  
    123123    imp->overflow = 0;
    124124
     125    if (!_needsMarking) {
     126        imp->valueRefCount = 1;
     127    }
    125128#if DUMP_STATISTICS
    126129    if (++numLists > numListsHighWaterMark)
    … …  
    130133}
    131134
    132 inline void List::derefValues()
     135List::List(bool needsMarking) : _impBase(allocateListImp()), _needsMarking(needsMarking)
     136{
     137    ListImp *imp = static_cast<ListImp *>(_impBase);
     138    imp->size = 0;
     139    imp->refCount = 1;
     140    imp->capacity = 0;
     141    imp->overflow = 0;
     142
     143    if (!_needsMarking) {
     144        imp->valueRefCount = 1;
     145    }
     146
     147#if DUMP_STATISTICS
     148    if (++numLists > numListsHighWaterMark)
     149        numListsHighWaterMark = numLists;
     150    imp->sizeHighWaterMark = 0;
     151#endif
     152}
     153
     154void List::derefValues()
    133155{
    134156    ListImp *imp = static_cast<ListImp *>(_impBase);
    … …  
    144166    for (int i = 0; i != overflowSize; ++i)
    145167        overflow[i]->deref();
     168}
     169
     170void List::refValues()
     171{
     172    ListImp *imp = static_cast<ListImp *>(_impBase);
     173   
     174    int size = imp->size;
     175   
     176    int inlineSize = MIN(size, inlineValuesSize);
     177    for (int i = 0; i != inlineSize; ++i)
     178        imp->values[i]->ref();
     179   
     180    int overflowSize = size - inlineSize;
     181    ValueImp **overflow = imp->overflow;
     182    for (int i = 0; i != overflowSize; ++i)
     183        overflow[i]->ref();
     184}
     185
     186void List::markValues()
     187{
     188    ListImp *imp = static_cast<ListImp *>(_impBase);
     189   
     190    int size = imp->size;
     191   
     192    int inlineSize = MIN(size, inlineValuesSize);
     193    for (int i = 0; i != inlineSize; ++i) {
     194        if (!imp->values[i]->marked()) {
     195            imp->values[i]->mark();
     196        }
     197    }
     198
     199    int overflowSize = size - inlineSize;
     200    ValueImp **overflow = imp->overflow;
     201    for (int i = 0; i != overflowSize; ++i) {
     202        if (!overflow[i]->marked()) {
     203            overflow[i]->mark();
     204        }
     205    }
    146206}
    147207
    … …  
    158218#endif
    159219
    160     derefValues();
    161220    delete [] imp->overflow;
    162221    deallocateListImp(imp);
    … …  
    175234void List::clear()
    176235{
    177     derefValues();
     236    if (_impBase->valueRefCount > 0) {
     237        derefValues();
     238    }
    178239    _impBase->size = 0;
    179240}
    … …  
    190251#endif
    191252
    192     v->ref();
     253    if (imp->valueRefCount > 0) {
     254        v->ref();
     255    }
    193256   
    194257    if (i < inlineValuesSize) {
    … …  
    212275}
    213276
    214 List List::copyTail() const
     277List List::copy() const
    215278{
    216279    List copy;
    … …  
    232295}
    233296
     297
     298List List::copyTail() const
     299{
     300    List copy;
     301
     302    ListImp *imp = static_cast<ListImp *>(_impBase);
     303
     304    int size = imp->size;
     305
     306    int inlineSize = MIN(size, inlineValuesSize);
     307    for (int i = 1; i != inlineSize; ++i)
     308        copy.append(imp->values[i]);
     309
     310    ValueImp **overflow = imp->overflow;
     311    int overflowSize = size - inlineSize;
     312    for (int i = 0; i != overflowSize; ++i)
     313        copy.append(overflow[i]);
     314
     315    return copy;
     316}
     317
    234318const List &List::empty()
    235319{

  • trunk/JavaScriptCore/kjs/list.h

    r2843 r2883  
    3131        int size;
    3232        int refCount;
     33        int valueRefCount;
    3334    };
    3435   
    … …  
    4849    public:
    4950        List();
     51        List(bool needsMarking);
    5052        ~List() { deref(); }
    5153
    52         List(const List &b) : _impBase(b._impBase) { ++_impBase->refCount; }
     54        List(const List &b) : _impBase(b._impBase), _needsMarking(false) {
     55            ++_impBase->refCount;
     56            if (!_impBase->valueRefCount) refValues();
     57            ++_impBase->valueRefCount;
     58        }
    5359        List &operator=(const List &);
    5460
    … …  
    6470         */
    6571        void clear();
     72
     73        /**
     74         * Make a copy of the list
     75         */
     76        List copy() const;
     77
    6678        /**
    6779         * Make a copy of the list, omitting the first element.
    … …  
    108120        static const List &empty();
    109121       
     122        void mark() { if (_impBase->valueRefCount == 0) markValues(); }
    110123    private:
    111124        ListImpBase *_impBase;
    112        
    113         void deref() { if (--_impBase->refCount == 0) release(); }
     125        bool _needsMarking;
     126       
     127        void deref() { if (!_needsMarking && --_impBase->valueRefCount == 0) derefValues(); if (--_impBase->refCount == 0) release(); }
    114128
    115129        void release();
     130        void refValues();
    116131        void derefValues();
     132        void markValues();
    117133    };
    118134 
    … …  
    177193        deref();
    178194        _impBase = bImpBase;
     195        if (!_needsMarking) {
     196            if (!_impBase->valueRefCount) {
     197                refValues();
     198            }
     199            _impBase->valueRefCount++;
     200        }
     201
    179202        return *this;
    180203    }

  • trunk/JavaScriptCore/kjs/value.cpp

    r2845 r2883  
    204204#if DEBUG_COLLECTOR
    205205  assert (!(rep && !SimpleNumber::is(rep) && *((uint32_t *)rep) == 0 ));
    206   assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & VI_MARKED));
     206  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & ValueImp::VI_MARKED));
    207207#endif
    208208  if (v)
    … …  
    219219#if DEBUG_COLLECTOR
    220220  assert (!(rep && !SimpleNumber::is(rep) && *((uint32_t *)rep) == 0 ));
    221   assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & VI_MARKED));
     221  assert (!(rep && !SimpleNumber::is(rep) && rep->_flags & ValueImp::VI_MARKED));
    222222#endif
    223223  if (rep)
Note: See TracChangeset for help on using the changeset viewer.