Changeset 11773 in webkit for trunk/JavaScriptCore


Ignore:
Timestamp:
Dec 27, 2005, 1:24:14 AM (19 years ago)
Author:
mjs
Message:

Reviewed by Darin and Geoff.

Changes by me and Anders.

  • also fixed some warnings reported by -Winline
  • JavaScriptCorePrefix.h: Move new and delete definitions higher so there aren't conflicts with use in standard C++ headers
  • kjs/object.cpp: (KJS::throwSetterError): Moved this piece of put into a seprate function to avoid the PIC branch. (KJS::JSObject::put): Use hasGetterSetterProperties to avoid expensive stuff when not needed. Also use GetterSetter properties attribute. (KJS::JSObject::deleteProperty): Recompute whether any properties are getter/setter properties any more, if this one was one. (KJS::JSObject::defineGetter): Let the PropertyMap know that it has getter/setter properties now (and use the new attribute). (KJS::JSObject::defineSetter): Ditto. (KJS::JSObject::fillGetterPropertySlot): Out-of-line helper for getOwnPropertySlot, to avoid global variable access in the hot code path.
  • kjs/object.h: (KJS::): Added GetterSetter attribute. (KJS::JSCell::isObject): Moved lower to be after inline methods it uses. (KJS::JSValue::isObject): ditto (KJS::JSObject::getOwnPropertySlot): try to avoid impact of getters and setters as much as possible in the case where they are not being used
  • kjs/property_map.cpp: (KJS::PropertyMap::containsGettersOrSetters): New method to help with this
  • kjs/property_map.h: (KJS::PropertyMap::hasGetterSetterProperties): Ditto (KJS::PropertyMap::setHasGetterSetterProperties): Ditto (KJS::PropertyMap::PropertyMap): Added a crazy hack to store the global "has getter/setter properties" flag in the property map single entry, to avoid making objects any bigger.
  • kjs/value.h: Moved some things to object.h to make -Winline happier
Location:
trunk/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r11763 r11773  
     12005-12-26  Maciej Stachowiak  <[email protected]>
     2
     3        Reviewed by Darin and Geoff.
     4
     5        Changes by me and Anders.
     6
     7        - mostly fixed REGRESSION: 5-10% performance regression on JS iBench from getter/setter change
     8        http://bugzilla.opendarwin.org/show_bug.cgi?id=6083
     9
     10        - also fixed some warnings reported by -Winline
     11       
     12        * JavaScriptCorePrefix.h: Move new and delete definitions higher so there
     13        aren't conflicts with use in standard C++ headers
     14        * kjs/object.cpp:
     15        (KJS::throwSetterError): Moved this piece of put into a seprate function
     16        to avoid the PIC branch.
     17        (KJS::JSObject::put): Use hasGetterSetterProperties to avoid expensive stuff
     18        when not needed. Also use GetterSetter properties attribute.
     19        (KJS::JSObject::deleteProperty): Recompute whether any properties are getter/setter
     20        properties any more, if this one was one.
     21        (KJS::JSObject::defineGetter): Let the PropertyMap know that it has getter/setter
     22        properties now (and use the new attribute).
     23        (KJS::JSObject::defineSetter): Ditto.
     24        (KJS::JSObject::fillGetterPropertySlot): Out-of-line helper for getOwnPropertySlot,
     25        to avoid global variable access in the hot code path.
     26        * kjs/object.h:
     27        (KJS::): Added GetterSetter attribute.
     28        (KJS::JSCell::isObject): Moved lower to be after inline methods it uses.
     29        (KJS::JSValue::isObject): ditto
     30        (KJS::JSObject::getOwnPropertySlot): try to avoid impact of getters and setters
     31        as much as possible in the case where they are not being used
     32        * kjs/property_map.cpp:
     33        (KJS::PropertyMap::containsGettersOrSetters): New method to help with this
     34        * kjs/property_map.h:
     35        (KJS::PropertyMap::hasGetterSetterProperties): Ditto
     36        (KJS::PropertyMap::setHasGetterSetterProperties): Ditto
     37        (KJS::PropertyMap::PropertyMap): Added a crazy hack to store the
     38        global "has getter/setter properties" flag in the property map
     39        single entry, to avoid making objects any bigger.
     40        * kjs/value.h: Moved some things to object.h to make -Winline happier
     41
    1422005-12-24  Maciej Stachowiak  <[email protected]>
    243

  • trunk/JavaScriptCore/JavaScriptCorePrefix.h

    r10701 r11773  
     1#ifdef __cplusplus
     2#define new ("if you use new/delete make sure to include config.h at the top of the file"())
     3#define delete ("if you use new/delete make sure to include config.h at the top of the file"())
     4#endif
     5
    16#ifdef __cplusplus
    27#define NULL __null
    … …  
    49#define NULL ((void *)0)
    510#endif
     11
     12#include "config.h"
    613
    714#include <assert.h>
    … …  
    4148#include <typeinfo>
    4249
    43 #ifdef __cplusplus
    44 #define new ("if you use new/delete make sure to include config.h at the top of the file"())
    45 #define delete ("if you use new/delete make sure to include config.h at the top of the file"())
    4650#endif
    47 #endif

  • trunk/JavaScriptCore/kjs/object.cpp

    r11566 r11773  
    191191}
    192192
     193static void throwSetterError(ExecState *exec)
     194{
     195  throwError(exec, TypeError, "setting a property that has only a getter");
     196}
     197
    193198// ECMA 8.6.2.2
    194199void JSObject::put(ExecState *exec, const Identifier &propertyName, JSValue *value, int attr)
    … …  
    216221  JSObject *obj = this;
    217222  while (true) {
    218     if (JSValue *gs = obj->_prop.get(propertyName)) {
    219       if (gs->type() == GetterSetterType) {
     223    JSValue *gs;
     224    int attributes;
     225    if (obj->_prop.hasGetterSetterProperties() && (gs = obj->_prop.get(propertyName, attributes))) {
     226      if (attributes & GetterSetter) {
    220227        JSObject *setterFunc = static_cast<GetterSetterImp *>(gs)->getSetter();
    221228           
    222229        if (!setterFunc) {
    223           throwError(exec, TypeError, "setting a property that has only a getter");
     230          throwSetterError(exec);
    224231          return;
    225232        }
    … …  
    237244    }
    238245     
    239     if (!obj->_proto || !obj->_proto->isObject())
     246    if (!obj->_proto->isObject())
    240247      break;
    241248       
    … …  
    288295      return false;
    289296    _prop.remove(propertyName);
     297    if (attributes & GetterSetter)
     298        _prop.setHasGetterSetterProperties(_prop.containsGettersOrSetters());
    290299    return true;
    291300  }
    … …  
    371380    } else {
    372381        gs = new GetterSetterImp;
    373         putDirect(propertyName, gs);
    374     }
    375    
     382        putDirect(propertyName, gs, GetterSetter);
     383    }
     384   
     385    _prop.setHasGetterSetterProperties(true);
    376386    gs->setGetter(getterFunc);
    377387}
    … …  
    387397        gs = new GetterSetterImp;
    388398        putDirect(propertyName, gs);
    389     }
    390    
     399        putDirect(propertyName, gs, GetterSetter);
     400    }
     401   
     402    _prop.setHasGetterSetterProperties(true);
    391403    gs->setSetter(setterFunc);
    392404}
    … …  
    519531{
    520532    _prop.put(propertyName, jsNumber(value), attr);
     533}
     534
     535void JSObject::fillGetterPropertySlot(PropertySlot& slot, JSValue **location)
     536{
     537    GetterSetterImp *gs = static_cast<GetterSetterImp *>(*location);
     538    JSObject *getterFunc = gs->getGetter();
     539    if (getterFunc)
     540        slot.setGetterSlot(this, getterFunc);
     541    else
     542        slot.setUndefined(this);
    521543}
    522544

  • trunk/JavaScriptCore/kjs/object.h

    r11566 r11773  
    5151  // ECMA 262-3 8.6.1
    5252  // Property attributes
    53   enum Attribute { None       = 0,
    54                    ReadOnly   = 1 << 1, // property can be only read, not written
    55                    DontEnum   = 1 << 2, // property doesn't appear in (for .. in ..)
    56                    DontDelete = 1 << 3, // property can't be deleted
    57                    Internal   = 1 << 4, // an internal property, set to bypass checks
    58                    Function   = 1 << 5 }; // property is a function - only used by static hashtables
     53  enum Attribute { None         = 0,
     54                   ReadOnly     = 1 << 1, // property can be only read, not written
     55                   DontEnum     = 1 << 2, // property doesn't appear in (for .. in ..)
     56                   DontDelete   = 1 << 3, // property can't be deleted
     57                   Internal     = 1 << 4, // an internal property, set to bypass checks
     58                   Function     = 1 << 5, // property is a function - only used by static hashtables
     59                   GetterSetter = 1 << 6 }; // property is a getter or setter
    5960
    6061  /**
    … …  
    503504    void putDirect(const Identifier &propertyName, JSValue *value, int attr = 0);
    504505    void putDirect(const Identifier &propertyName, int value, int attr = 0);
    505    
     506
     507    void fillGetterPropertySlot(PropertySlot& slot, JSValue **location);
     508
    506509    void defineGetter(ExecState *exec, const Identifier& propertyName, JSObject *getterFunc);
    507510    void defineSetter(ExecState *exec, const Identifier& propertyName, JSObject *setterFunc);
    … …  
    567570JSObject *throwError(ExecState *, ErrorType, const char *message);
    568571JSObject *throwError(ExecState *, ErrorType);
    569  
    570 inline bool JSCell::isObject(const ClassInfo *info) const
    571 {
    572     return isObject() && static_cast<const JSObject *>(this)->inherits(info);
    573 }
    574572
    575573inline JSObject::JSObject(JSObject *proto)
    … …  
    613611}
    614612
     613// this method is here to be after the inline declaration of JSObject::inherits
     614inline bool JSCell::isObject(const ClassInfo *info) const
     615{
     616    return isObject() && static_cast<const JSObject *>(this)->inherits(info);
     617}
     618
     619// this method is here to be after the inline declaration of JSCell::isObject
     620inline bool JSValue::isObject(const ClassInfo *c) const
     621{
     622    return !SimpleNumber::is(this) && downcast()->isObject(c);
     623}
     624
    615625// It may seem crazy to inline a function this large but it makes a big difference
    616626// since this is function very hot in variable lookup
    … …  
    633643// but it makes a big difference to property lookup that derived classes can inline their
    634644// base class call to this.
    635 inline bool JSObject::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
     645ALWAYS_INLINE bool JSObject::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
    636646{
    637647    if (JSValue **location = getDirectLocation(propertyName)) {
    638         if ((*location)->type() == GetterSetterType) {
    639             GetterSetterImp *gs = static_cast<GetterSetterImp *>(*location);
    640             JSObject *getterFunc = gs->getGetter();
    641             if (getterFunc)
    642                 slot.setGetterSlot(this, getterFunc);
    643             else
    644                 slot.setUndefined(this);
    645         } else {
     648        if (_prop.hasGetterSetterProperties() && location[0]->type() == GetterSetterType)
     649            fillGetterPropertySlot(slot, location);
     650        else
    646651            slot.setValueSlot(this, location);
    647         }
    648652        return true;
    649653    }

  • trunk/JavaScriptCore/kjs/property_map.cpp

    r11763 r11773  
    567567}
    568568
     569bool PropertyMap::containsGettersOrSetters() const
     570{
     571    if (!_table) {
     572#if USE_SINGLE_ENTRY
     573        return _singleEntry.attributes & GetterSetter;
     574#endif
     575        return false;
     576    }
     577
     578    for (int i = 0; i != _table->size; ++i) {
     579        if (_table->entries[i].attributes & GetterSetter)
     580            return true;
     581    }
     582   
     583    return false;
     584}
     585
    569586void PropertyMap::addEnumerablesToReferenceList(ReferenceList &list, JSObject *base) const
    570587{

  • trunk/JavaScriptCore/kjs/property_map.h

    r11527 r11773  
    6161        UString::Rep *key;
    6262        JSValue *value;
    63         int attributes;
     63        short attributes;
     64        short globalGetterSetterFlag;
    6465        int index;
    6566    };
    … …  
    8889        void restore(const SavedProperties &p);
    8990
     91        bool hasGetterSetterProperties() const { return _singleEntry.globalGetterSetterFlag; }
     92        void setHasGetterSetterProperties(bool f) { _singleEntry.globalGetterSetterFlag = f; }
     93
     94        bool containsGettersOrSetters() const;
    9095    private:
    9196        static bool keysMatch(const UString::Rep *, const UString::Rep *);
    … …  
    106111    };
    107112
    108 inline PropertyMap::PropertyMap() : _table(NULL)
     113inline PropertyMap::PropertyMap() : _table(0)
    109114{
     115    _singleEntry.globalGetterSetterFlag = 0;
    110116}
    111117

  • trunk/JavaScriptCore/kjs/value.h

    r11566 r11773  
    314314}
    315315
    316 inline bool JSValue::isObject(const ClassInfo *c) const
    317 {
    318     return !SimpleNumber::is(this) && downcast()->isObject(c);
    319 }
    320 
    321316inline bool JSValue::getBoolean(bool& v) const
    322317{
Note: See TracChangeset for help on using the changeset viewer.