[webkit-changes] cvs commit: JavaScriptCore/kjs object.cpp object.h
property_map.cpp property_map.h value.h
Maciej
mjs at opensource.apple.com
Tue Dec 27 01:24:16 PST 2005
mjs 05/12/27 01:24:15
Modified: . ChangeLog JavaScriptCorePrefix.h
kjs object.cpp object.h property_map.cpp property_map.h
value.h
Log:
Reviewed by Darin and Geoff.
Changes by me and Anders.
- mostly fixed REGRESSION: 5-10% performance regression on JS iBench from getter/setter change
http://bugzilla.opendarwin.org/show_bug.cgi?id=6083
- 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
Revision Changes Path
1.930 +41 -0 JavaScriptCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/JavaScriptCore/ChangeLog,v
retrieving revision 1.929
retrieving revision 1.930
diff -u -r1.929 -r1.930
--- ChangeLog 25 Dec 2005 09:22:32 -0000 1.929
+++ ChangeLog 27 Dec 2005 09:24:11 -0000 1.930
@@ -1,3 +1,44 @@
+2005-12-26 Maciej Stachowiak <mjs at apple.com>
+
+ Reviewed by Darin and Geoff.
+
+ Changes by me and Anders.
+
+ - mostly fixed REGRESSION: 5-10% performance regression on JS iBench from getter/setter change
+ http://bugzilla.opendarwin.org/show_bug.cgi?id=6083
+
+ - 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
+
2005-12-24 Maciej Stachowiak <mjs at apple.com>
Reviewed by Eric and Dave Hyatt.
1.7 +7 -4 JavaScriptCore/JavaScriptCorePrefix.h
Index: JavaScriptCorePrefix.h
===================================================================
RCS file: /cvs/root/JavaScriptCore/JavaScriptCorePrefix.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- JavaScriptCorePrefix.h 3 Oct 2005 21:11:39 -0000 1.6
+++ JavaScriptCorePrefix.h 27 Dec 2005 09:24:12 -0000 1.7
@@ -1,9 +1,16 @@
#ifdef __cplusplus
+#define new ("if you use new/delete make sure to include config.h at the top of the file"())
+#define delete ("if you use new/delete make sure to include config.h at the top of the file"())
+#endif
+
+#ifdef __cplusplus
#define NULL __null
#else
#define NULL ((void *)0)
#endif
+#include "config.h"
+
#include <assert.h>
#include <ctype.h>
#include <float.h>
@@ -40,8 +47,4 @@
#include <list>
#include <typeinfo>
-#ifdef __cplusplus
-#define new ("if you use new/delete make sure to include config.h at the top of the file"())
-#define delete ("if you use new/delete make sure to include config.h at the top of the file"())
-#endif
#endif
1.60 +27 -5 JavaScriptCore/kjs/object.cpp
Index: object.cpp
===================================================================
RCS file: /cvs/root/JavaScriptCore/kjs/object.cpp,v
retrieving revision 1.59
retrieving revision 1.60
diff -u -r1.59 -r1.60
--- object.cpp 13 Dec 2005 21:24:52 -0000 1.59
+++ object.cpp 27 Dec 2005 09:24:13 -0000 1.60
@@ -190,6 +190,11 @@
return getOwnPropertySlot(exec, Identifier::from(propertyName), slot);
}
+static void throwSetterError(ExecState *exec)
+{
+ throwError(exec, TypeError, "setting a property that has only a getter");
+}
+
// ECMA 8.6.2.2
void JSObject::put(ExecState *exec, const Identifier &propertyName, JSValue *value, int attr)
{
@@ -215,12 +220,14 @@
JSObject *obj = this;
while (true) {
- if (JSValue *gs = obj->_prop.get(propertyName)) {
- if (gs->type() == GetterSetterType) {
+ JSValue *gs;
+ int attributes;
+ if (obj->_prop.hasGetterSetterProperties() && (gs = obj->_prop.get(propertyName, attributes))) {
+ if (attributes & GetterSetter) {
JSObject *setterFunc = static_cast<GetterSetterImp *>(gs)->getSetter();
if (!setterFunc) {
- throwError(exec, TypeError, "setting a property that has only a getter");
+ throwSetterError(exec);
return;
}
@@ -236,7 +243,7 @@
}
}
- if (!obj->_proto || !obj->_proto->isObject())
+ if (!obj->_proto->isObject())
break;
obj = static_cast<JSObject *>(obj->_proto);
@@ -287,6 +294,8 @@
if ((attributes & DontDelete))
return false;
_prop.remove(propertyName);
+ if (attributes & GetterSetter)
+ _prop.setHasGetterSetterProperties(_prop.containsGettersOrSetters());
return true;
}
@@ -370,9 +379,10 @@
gs = static_cast<GetterSetterImp *>(o);
} else {
gs = new GetterSetterImp;
- putDirect(propertyName, gs);
+ putDirect(propertyName, gs, GetterSetter);
}
+ _prop.setHasGetterSetterProperties(true);
gs->setGetter(getterFunc);
}
@@ -386,8 +396,10 @@
} else {
gs = new GetterSetterImp;
putDirect(propertyName, gs);
+ putDirect(propertyName, gs, GetterSetter);
}
+ _prop.setHasGetterSetterProperties(true);
gs->setSetter(setterFunc);
}
@@ -520,6 +532,16 @@
_prop.put(propertyName, jsNumber(value), attr);
}
+void JSObject::fillGetterPropertySlot(PropertySlot& slot, JSValue **location)
+{
+ GetterSetterImp *gs = static_cast<GetterSetterImp *>(*location);
+ JSObject *getterFunc = gs->getGetter();
+ if (getterFunc)
+ slot.setGetterSlot(this, getterFunc);
+ else
+ slot.setUndefined(this);
+}
+
// ------------------------------ Error ----------------------------------------
const char * const errorNamesArr[] = {
1.52 +26 -22 JavaScriptCore/kjs/object.h
Index: object.h
===================================================================
RCS file: /cvs/root/JavaScriptCore/kjs/object.h,v
retrieving revision 1.51
retrieving revision 1.52
diff -u -r1.51 -r1.52
--- object.h 13 Dec 2005 21:24:52 -0000 1.51
+++ object.h 27 Dec 2005 09:24:13 -0000 1.52
@@ -50,12 +50,13 @@
// ECMA 262-3 8.6.1
// Property attributes
- enum Attribute { None = 0,
- ReadOnly = 1 << 1, // property can be only read, not written
- DontEnum = 1 << 2, // property doesn't appear in (for .. in ..)
- DontDelete = 1 << 3, // property can't be deleted
- Internal = 1 << 4, // an internal property, set to bypass checks
- Function = 1 << 5 }; // property is a function - only used by static hashtables
+ enum Attribute { None = 0,
+ ReadOnly = 1 << 1, // property can be only read, not written
+ DontEnum = 1 << 2, // property doesn't appear in (for .. in ..)
+ DontDelete = 1 << 3, // property can't be deleted
+ Internal = 1 << 4, // an internal property, set to bypass checks
+ Function = 1 << 5, // property is a function - only used by static hashtables
+ GetterSetter = 1 << 6 }; // property is a getter or setter
/**
* Class Information
@@ -502,7 +503,9 @@
{ return _prop.getLocation(propertyName); }
void putDirect(const Identifier &propertyName, JSValue *value, int attr = 0);
void putDirect(const Identifier &propertyName, int value, int attr = 0);
-
+
+ void fillGetterPropertySlot(PropertySlot& slot, JSValue **location);
+
void defineGetter(ExecState *exec, const Identifier& propertyName, JSObject *getterFunc);
void defineSetter(ExecState *exec, const Identifier& propertyName, JSObject *setterFunc);
@@ -566,11 +569,6 @@
JSObject *throwError(ExecState *, ErrorType, const UString &message);
JSObject *throwError(ExecState *, ErrorType, const char *message);
JSObject *throwError(ExecState *, ErrorType);
-
-inline bool JSCell::isObject(const ClassInfo *info) const
-{
- return isObject() && static_cast<const JSObject *>(this)->inherits(info);
-}
inline JSObject::JSObject(JSObject *proto)
: _proto(proto), _internalValue(0)
@@ -612,6 +610,18 @@
return false;
}
+// this method is here to be after the inline declaration of JSObject::inherits
+inline bool JSCell::isObject(const ClassInfo *info) const
+{
+ return isObject() && static_cast<const JSObject *>(this)->inherits(info);
+}
+
+// this method is here to be after the inline declaration of JSCell::isObject
+inline bool JSValue::isObject(const ClassInfo *c) const
+{
+ return !SimpleNumber::is(this) && downcast()->isObject(c);
+}
+
// It may seem crazy to inline a function this large but it makes a big difference
// since this is function very hot in variable lookup
inline bool JSObject::getPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
@@ -632,19 +642,13 @@
// It may seem crazy to inline a function this large, especially a virtual function,
// but it makes a big difference to property lookup that derived classes can inline their
// base class call to this.
-inline bool JSObject::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
+ALWAYS_INLINE bool JSObject::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
{
if (JSValue **location = getDirectLocation(propertyName)) {
- if ((*location)->type() == GetterSetterType) {
- GetterSetterImp *gs = static_cast<GetterSetterImp *>(*location);
- JSObject *getterFunc = gs->getGetter();
- if (getterFunc)
- slot.setGetterSlot(this, getterFunc);
- else
- slot.setUndefined(this);
- } else {
+ if (_prop.hasGetterSetterProperties() && location[0]->type() == GetterSetterType)
+ fillGetterPropertySlot(slot, location);
+ else
slot.setValueSlot(this, location);
- }
return true;
}
1.57 +17 -0 JavaScriptCore/kjs/property_map.cpp
Index: property_map.cpp
===================================================================
RCS file: /cvs/root/JavaScriptCore/kjs/property_map.cpp,v
retrieving revision 1.56
retrieving revision 1.57
diff -u -r1.56 -r1.57
--- property_map.cpp 25 Dec 2005 09:22:34 -0000 1.56
+++ property_map.cpp 27 Dec 2005 09:24:14 -0000 1.57
@@ -566,6 +566,23 @@
return 0;
}
+bool PropertyMap::containsGettersOrSetters() const
+{
+ if (!_table) {
+#if USE_SINGLE_ENTRY
+ return _singleEntry.attributes & GetterSetter;
+#endif
+ return false;
+ }
+
+ for (int i = 0; i != _table->size; ++i) {
+ if (_table->entries[i].attributes & GetterSetter)
+ return true;
+ }
+
+ return false;
+}
+
void PropertyMap::addEnumerablesToReferenceList(ReferenceList &list, JSObject *base) const
{
if (!_table) {
1.28 +8 -2 JavaScriptCore/kjs/property_map.h
Index: property_map.h
===================================================================
RCS file: /cvs/root/JavaScriptCore/kjs/property_map.h,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- property_map.h 11 Dec 2005 02:05:47 -0000 1.27
+++ property_map.h 27 Dec 2005 09:24:14 -0000 1.28
@@ -60,7 +60,8 @@
PropertyMapHashTableEntry() : key(0) { }
UString::Rep *key;
JSValue *value;
- int attributes;
+ short attributes;
+ short globalGetterSetterFlag;
int index;
};
/**
@@ -87,6 +88,10 @@
void save(SavedProperties &) const;
void restore(const SavedProperties &p);
+ bool hasGetterSetterProperties() const { return _singleEntry.globalGetterSetterFlag; }
+ void setHasGetterSetterProperties(bool f) { _singleEntry.globalGetterSetterFlag = f; }
+
+ bool containsGettersOrSetters() const;
private:
static bool keysMatch(const UString::Rep *, const UString::Rep *);
void expand();
@@ -105,8 +110,9 @@
Entry _singleEntry;
};
-inline PropertyMap::PropertyMap() : _table(NULL)
+inline PropertyMap::PropertyMap() : _table(0)
{
+ _singleEntry.globalGetterSetterFlag = 0;
}
} // namespace
1.42 +0 -5 JavaScriptCore/kjs/value.h
Index: value.h
===================================================================
RCS file: /cvs/root/JavaScriptCore/kjs/value.h,v
retrieving revision 1.41
retrieving revision 1.42
diff -u -r1.41 -r1.42
--- value.h 13 Dec 2005 21:24:53 -0000 1.41
+++ value.h 27 Dec 2005 09:24:14 -0000 1.42
@@ -313,11 +313,6 @@
return !SimpleNumber::is(this) && downcast()->isObject();
}
-inline bool JSValue::isObject(const ClassInfo *c) const
-{
- return !SimpleNumber::is(this) && downcast()->isObject(c);
-}
-
inline bool JSValue::getBoolean(bool& v) const
{
return !SimpleNumber::is(this) && downcast()->getBoolean(v);
More information about the webkit-changes
mailing list