2008-09-09 Sam Weinig <sam@webkit.org>

        Reviewed by Geoffrey Garen.

        Replace uses of PropertyMap::get and PropertyMap::getLocation with
        PropertyMap::getOffset.

        Sunspider reports this as a .6% improvement.

        * JavaScriptCore.exp:
        * kjs/JSObject.cpp:
        (JSC::JSObject::put):
        (JSC::JSObject::deleteProperty):
        (JSC::JSObject::getPropertyAttributes):
        * kjs/JSObject.h:
        (JSC::JSObject::getDirect):
        (JSC::JSObject::getDirectLocation):
        (JSC::JSObject::locationForOffset):
        * kjs/PropertyMap.cpp:
        (JSC::PropertyMap::remove):
        (JSC::PropertyMap::getOffset):
        * kjs/PropertyMap.h:

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 331d6ea9038bb50717f739d3f50d4d94874e3502..d50863eaef7e556104ffff6afec257da23290a14 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2008-09-09  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Geoffrey Garen.
+
+        Replace uses of PropertyMap::get and PropertyMap::getLocation with
+        PropertyMap::getOffset.
+
+        Sunspider reports this as a .6% improvement.
+
+        * JavaScriptCore.exp:
+        * kjs/JSObject.cpp:
+        (JSC::JSObject::put):
+        (JSC::JSObject::deleteProperty):
+        (JSC::JSObject::getPropertyAttributes):
+        * kjs/JSObject.h:
+        (JSC::JSObject::getDirect):
+        (JSC::JSObject::getDirectLocation):
+        (JSC::JSObject::locationForOffset):
+        * kjs/PropertyMap.cpp:
+        (JSC::PropertyMap::remove):
+        (JSC::PropertyMap::getOffset):
+        * kjs/PropertyMap.h:
+
 2008-09-09  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
 
         Reviewed by Sam Weinig.

diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index b805b0b50ee7cfb396e3ab52b27df2c2d86afd43..8d5e961557bec7b1e82cb723a1673b0586102183 100644 (file)
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -103,9 +103,8 @@ __ZN3JSC11JSImmediate8toStringEPKNS_7JSValueE
 __ZN3JSC11JSImmediate9prototypeEPKNS_7JSValueEPNS_9ExecStateE
 __ZN3JSC11ProfileNode4sortEPFbRKN3WTF6RefPtrIS0_EES5_E
 __ZN3JSC11ProgramNode6createEPNS_12JSGlobalDataEPNS_14SourceElementsEPN3WTF6VectorISt4pairINS_10IdentifierEjELm16EEEPNS6_INS5_6RefPtrINS_12FuncDeclNodeEEELm16EEEPNS_14SourceProviderEbbi
-__ZN3JSC11PropertyMap11getLocationERKNS_10IdentifierERKN3WTF11OwnArrayPtrIPNS_7JSValueEEE
-__ZN3JSC11PropertyMap11getLocationERKNS_10IdentifierERbRKN3WTF11OwnArrayPtrIPNS_7JSValueEEE
 __ZN3JSC11PropertyMap3putERKNS_10IdentifierEPNS_7JSValueEjbPNS_8JSObjectERNS_15PutPropertySlotERN3WTF11OwnArrayPtrIS5_EE
+__ZN3JSC11PropertyMap9getOffsetERKNS_10IdentifierE
 __ZN3JSC11PropertyMap9getOffsetERKNS_10IdentifierERb
 __ZN3JSC11PropertyMapD1Ev
 __ZN3JSC11StructureID21addPropertyTransitionEPS0_RKNS_10IdentifierEPNS_7JSValueEjbPNS_8JSObjectERNS_15PutPropertySlotERN3WTF11OwnArrayPtrIS6_EE
@@ -280,7 +279,6 @@ __ZN3WTF8Collator18setOrderLowerFirstEb
 __ZN3WTF8CollatorC1EPKc
 __ZN3WTF8CollatorD1Ev
 __ZN3WTF8fastFreeEPv
-__ZNK3JSC11PropertyMap3getERKNS_10IdentifierERKN3WTF11OwnArrayPtrIPNS_7JSValueEEE
 __ZNK3JSC12DateInstance7getTimeERdRi
 __ZNK3JSC12StringObject12toThisStringEPNS_9ExecStateE
 __ZNK3JSC12StringObject8toStringEPNS_9ExecStateE

diff --git a/JavaScriptCore/kjs/JSObject.cpp b/JavaScriptCore/kjs/JSObject.cpp
index bafdb0c19ddbb21b33d3d384d8cc61182c2875d6..d43a6552c568158a98eda3b8e2ea85234e6766b2 100644 (file)
--- a/JavaScriptCore/kjs/JSObject.cpp
+++ b/JavaScriptCore/kjs/JSObject.cpp
@@ -136,11 +136,11 @@ void JSObject::put(ExecState* exec, const Identifier& propertyName, JSValue* val
     }
     
     unsigned attributes;
-    if (m_structureID->propertyMap().get(propertyName, attributes, m_propertyStorage) && attributes & ReadOnly)
+    if ((m_structureID->propertyMap().getOffset(propertyName, attributes) != WTF::notFound) && attributes & ReadOnly)
         return;
 
     for (JSObject* obj = this; ; obj = static_cast<JSObject*>(prototype)) {
-        if (JSValue* gs = obj->structureID()->propertyMap().get(propertyName, obj->propertyStorage())) {
+        if (JSValue* gs = obj->getDirect(propertyName)) {
             if (gs->isGetterSetter()) {
                 JSObject* setterFunc = static_cast<GetterSetter*>(gs)->setter();        
                 if (!setterFunc) {
@@ -202,8 +202,7 @@ bool JSObject::hasProperty(ExecState* exec, unsigned propertyName) const
 bool JSObject::deleteProperty(ExecState* exec, const Identifier& propertyName)
 {
     unsigned attributes;
-    JSValue* v = m_structureID->propertyMap().get(propertyName, attributes, m_propertyStorage);
-    if (v) {
+    if (m_structureID->propertyMap().getOffset(propertyName, attributes) != WTF::notFound) {
         if ((attributes & DontDelete))
             return false;
         removeDirect(propertyName);
@@ -421,7 +420,7 @@ bool JSObject::propertyIsEnumerable(ExecState* exec, const Identifier& propertyN
 
 bool JSObject::getPropertyAttributes(ExecState* exec, const Identifier& propertyName, unsigned& attributes) const
 {
-    if (m_structureID->propertyMap().get(propertyName, attributes, m_propertyStorage))
+    if (m_structureID->propertyMap().getOffset(propertyName, attributes) != WTF::notFound)
         return true;
     
     // Look in the static hashtable of properties

diff --git a/JavaScriptCore/kjs/JSObject.h b/JavaScriptCore/kjs/JSObject.h
index 498f00edd22612f0959233d3e7a7a4c3425fb4bc..e4b77e336acbd4d484f8b97f132796ddadc9dd34 100644 (file)
--- a/JavaScriptCore/kjs/JSObject.h
+++ b/JavaScriptCore/kjs/JSObject.h
@@ -124,17 +124,20 @@ namespace JSC {
         // This get function only looks at the property map.
         JSValue* getDirect(const Identifier& propertyName) const
         {
-            return m_structureID->propertyMap().get(propertyName, m_propertyStorage);
+            size_t offset = m_structureID->propertyMap().getOffset(propertyName);
+            return offset != WTF::notFound ? m_propertyStorage[offset] : 0;
         }
 
         JSValue** getDirectLocation(const Identifier& propertyName)
         {
-            return m_structureID->propertyMap().getLocation(propertyName, m_propertyStorage);
+            size_t offset = m_structureID->propertyMap().getOffset(propertyName);
+            return offset != WTF::notFound ? locationForOffset(offset) : 0;
         }
 
         JSValue** getDirectLocation(const Identifier& propertyName, bool& isWriteable)
         {
-            return m_structureID->propertyMap().getLocation(propertyName, isWriteable, m_propertyStorage);
+            size_t offset = m_structureID->propertyMap().getOffset(propertyName, isWriteable);
+            return offset != WTF::notFound ? locationForOffset(offset) : 0;
         }
 
         size_t offsetForLocation(JSValue** location)
@@ -142,6 +145,11 @@ namespace JSC {
             return location - m_propertyStorage.get();
         }
 
+        JSValue** locationForOffset(size_t offset)
+        {
+            return &m_propertyStorage[offset];
+        }
+
         void removeDirect(const Identifier& propertyName);
         bool hasCustomProperties() { return !m_structureID->propertyMap().isEmpty(); }
         bool hasGetterSetterProperties() { return m_structureID->propertyMap().hasGetterSetterProperties(); }

diff --git a/JavaScriptCore/kjs/PropertyMap.cpp b/JavaScriptCore/kjs/PropertyMap.cpp
index 9e41077f014d74f6710dff2390b9c585fdccca3f..19d91fea2fc3402a317f1da7775ebae5872add50 100644 (file)
--- a/JavaScriptCore/kjs/PropertyMap.cpp
+++ b/JavaScriptCore/kjs/PropertyMap.cpp
@@ -117,190 +117,6 @@ PropertyMap::~PropertyMap()
     fastFree(m_table);
 }
 
-JSValue* PropertyMap::get(const Identifier& propertyName, unsigned& attributes, const PropertyStorage& propertyStorage) const
-{
-    ASSERT(!propertyName.isNull());
-
-    UString::Rep* rep = propertyName._ustring.rep();
-
-    if (!m_table)
-        return 0;
-
-    unsigned i = rep->computedHash();
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
-#endif
-
-    unsigned entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-    if (entryIndex == emptyEntryIndex)
-        return 0;
-
-    if (rep == m_table->entries()[entryIndex - 1].key) {
-        attributes = m_table->entries()[entryIndex - 1].attributes;
-        return propertyStorage[entryIndex - 1];
-    }
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numCollisions;
-#endif
-
-    unsigned k = 1 | doubleHash(rep->computedHash());
-
-    while (1) {
-        i += k;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
-
-        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-        if (entryIndex == emptyEntryIndex)
-            return 0;
-
-        if (rep == m_table->entries()[entryIndex - 1].key) {
-            attributes = m_table->entries()[entryIndex - 1].attributes;
-            return propertyStorage[entryIndex - 1];
-        }
-    }
-}
-
-JSValue* PropertyMap::get(const Identifier& propertyName, const PropertyStorage& propertyStorage) const
-{
-    ASSERT(!propertyName.isNull());
-
-    UString::Rep* rep = propertyName._ustring.rep();
-
-    if (!m_table)
-        return 0;
-
-    unsigned i = rep->computedHash();
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
-#endif
-
-    unsigned entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-    if (entryIndex == emptyEntryIndex)
-        return 0;
-
-    if (rep == m_table->entries()[entryIndex - 1].key)
-        return propertyStorage[entryIndex - 1];
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numCollisions;
-#endif
-
-    unsigned k = 1 | doubleHash(rep->computedHash());
-
-    while (1) {
-        i += k;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
-
-        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-        if (entryIndex == emptyEntryIndex)
-            return 0;
-
-        if (rep == m_table->entries()[entryIndex - 1].key)
-            return propertyStorage[entryIndex - 1];
-    }
-}
-
-JSValue** PropertyMap::getLocation(const Identifier& propertyName, const PropertyStorage& propertyStorage)
-{
-    ASSERT(!propertyName.isNull());
-
-    UString::Rep* rep = propertyName._ustring.rep();
-
-    if (!m_table)
-        return 0;
-
-    unsigned i = rep->computedHash();
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
-#endif
-
-    unsigned entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-    if (entryIndex == emptyEntryIndex)
-        return 0;
-
-    if (rep == m_table->entries()[entryIndex - 1].key)
-        return &propertyStorage[entryIndex - 1];
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numCollisions;
-#endif
-
-    unsigned k = 1 | doubleHash(rep->computedHash());
-
-    while (1) {
-        i += k;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
-
-        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-        if (entryIndex == emptyEntryIndex)
-            return 0;
-
-        if (rep == m_table->entries()[entryIndex - 1].key)
-            return &propertyStorage[entryIndex - 1];
-    }
-}
-
-JSValue** PropertyMap::getLocation(const Identifier& propertyName, bool& isWriteable, const PropertyStorage& propertyStorage)
-{
-    ASSERT(!propertyName.isNull());
-
-    UString::Rep* rep = propertyName._ustring.rep();
-
-    if (!m_table)
-        return 0;
-
-    unsigned i = rep->computedHash();
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
-#endif
-
-    unsigned entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-    if (entryIndex == emptyEntryIndex)
-        return 0;
-
-    if (rep == m_table->entries()[entryIndex - 1].key) {
-        isWriteable = !(m_table->entries()[entryIndex - 1].attributes & ReadOnly);
-        return &propertyStorage[entryIndex - 1];
-    }
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numCollisions;
-#endif
-
-    unsigned k = 1 | doubleHash(rep->computedHash());
-
-    while (1) {
-        i += k;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
-
-        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-        if (entryIndex == emptyEntryIndex)
-            return 0;
-
-        if (rep == m_table->entries()[entryIndex - 1].key) {
-            isWriteable = !(m_table->entries()[entryIndex - 1].attributes & ReadOnly);
-            return &propertyStorage[entryIndex - 1];
-        }
-    }
-}
-
 void PropertyMap::put(const Identifier& propertyName, JSValue* value, unsigned attributes, bool checkReadOnly, JSObject* slotBase, PutPropertySlot& slot, PropertyStorage& propertyStorage)
 {
     ASSERT(!propertyName.isNull());
@@ -391,6 +207,69 @@ void PropertyMap::put(const Identifier& propertyName, JSValue* value, unsigned a
     slot.setNewProperty(slotBase, entryIndex - 1);
 }
 
+void PropertyMap::remove(const Identifier& propertyName, PropertyStorage& propertyStorage)
+{
+    ASSERT(!propertyName.isNull());
+
+    checkConsistency(propertyStorage);
+
+    UString::Rep* rep = propertyName._ustring.rep();
+
+    if (!m_table)
+        return;
+
+#if DUMP_PROPERTYMAP_STATS
+    ++numProbes;
+    ++numRemoves;
+#endif
+
+    // Find the thing to remove.
+    unsigned i = rep->computedHash();
+    unsigned k = 0;
+    unsigned entryIndex;
+    UString::Rep* key = 0;
+    while (1) {
+        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
+        if (entryIndex == emptyEntryIndex)
+            return;
+
+        key = m_table->entries()[entryIndex - 1].key;
+        if (rep == key)
+            break;
+
+        if (k == 0) {
+            k = 1 | doubleHash(rep->computedHash());
+#if DUMP_PROPERTYMAP_STATS
+            ++numCollisions;
+#endif
+        }
+
+        i += k;
+
+#if DUMP_PROPERTYMAP_STATS
+        ++numRehashes;
+#endif
+    }
+
+    // Replace this one element with the deleted sentinel. Also clear out
+    // the entry so we can iterate all the entries as needed.
+    m_table->entryIndices[i & m_table->sizeMask] = deletedSentinelIndex;
+    key->deref();
+    m_table->entries()[entryIndex - 1].key = 0;
+    m_table->entries()[entryIndex - 1].attributes = 0;
+
+    propertyStorage[entryIndex - 1] = jsUndefined();
+
+    ASSERT(m_table->keyCount >= 1);
+    --m_table->keyCount;
+    ++m_table->deletedSentinelCount;
+
+    if (m_table->deletedSentinelCount * 4 >= m_table->size)
+        rehash(propertyStorage);
+
+    checkConsistency(propertyStorage);
+}
+
 size_t PropertyMap::getOffset(const Identifier& propertyName)
 {
     ASSERT(!propertyName.isNull());
@@ -435,6 +314,54 @@ size_t PropertyMap::getOffset(const Identifier& propertyName)
     }
 }
 
+size_t PropertyMap::getOffset(const Identifier& propertyName, unsigned& attributes)
+{
+    ASSERT(!propertyName.isNull());
+
+    if (!m_table)
+        return WTF::notFound;
+
+    UString::Rep* rep = propertyName._ustring.rep();
+
+    unsigned i = rep->computedHash();
+
+#if DUMP_PROPERTYMAP_STATS
+    ++numProbes;
+#endif
+
+    unsigned entryIndex = m_table->entryIndices[i & m_table->sizeMask];
+    if (entryIndex == emptyEntryIndex)
+        return WTF::notFound;
+
+    if (rep == m_table->entries()[entryIndex - 1].key) {
+        attributes = m_table->entries()[entryIndex - 1].attributes;
+        return entryIndex - 1;
+    }
+
+#if DUMP_PROPERTYMAP_STATS
+    ++numCollisions;
+#endif
+
+    unsigned k = 1 | doubleHash(rep->computedHash());
+
+    while (1) {
+        i += k;
+
+#if DUMP_PROPERTYMAP_STATS
+        ++numRehashes;
+#endif
+
+        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
+        if (entryIndex == emptyEntryIndex)
+            return WTF::notFound;
+
+        if (rep == m_table->entries()[entryIndex - 1].key) {
+            attributes = m_table->entries()[entryIndex - 1].attributes;
+            return entryIndex - 1;
+        }
+    }
+}
+
 size_t PropertyMap::getOffset(const Identifier& propertyName, bool& isWriteable)
 {
     ASSERT(!propertyName.isNull());
@@ -604,69 +531,6 @@ void PropertyMap::resizePropertyStorage(PropertyStorage& propertyStorage, unsign
     checkConsistency(propertyStorage);
 }
 
-void PropertyMap::remove(const Identifier& propertyName, PropertyStorage& propertyStorage)
-{
-    ASSERT(!propertyName.isNull());
-
-    checkConsistency(propertyStorage);
-
-    UString::Rep* rep = propertyName._ustring.rep();
-
-    if (!m_table)
-        return;
-
-#if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
-    ++numRemoves;
-#endif
-
-    // Find the thing to remove.
-    unsigned i = rep->computedHash();
-    unsigned k = 0;
-    unsigned entryIndex;
-    UString::Rep* key = 0;
-    while (1) {
-        entryIndex = m_table->entryIndices[i & m_table->sizeMask];
-        if (entryIndex == emptyEntryIndex)
-            return;
-
-        key = m_table->entries()[entryIndex - 1].key;
-        if (rep == key)
-            break;
-
-        if (k == 0) {
-            k = 1 | doubleHash(rep->computedHash());
-#if DUMP_PROPERTYMAP_STATS
-            ++numCollisions;
-#endif
-        }
-
-        i += k;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
-    }
-
-    // Replace this one element with the deleted sentinel. Also clear out
-    // the entry so we can iterate all the entries as needed.
-    m_table->entryIndices[i & m_table->sizeMask] = deletedSentinelIndex;
-    key->deref();
-    m_table->entries()[entryIndex - 1].key = 0;
-    m_table->entries()[entryIndex - 1].attributes = 0;
-
-    propertyStorage[entryIndex - 1] = jsUndefined();
-
-    ASSERT(m_table->keyCount >= 1);
-    --m_table->keyCount;
-    ++m_table->deletedSentinelCount;
-
-    if (m_table->deletedSentinelCount * 4 >= m_table->size)
-        rehash(propertyStorage);
-
-    checkConsistency(propertyStorage);
-}
-
 static int comparePropertyMapEntryIndices(const void* a, const void* b)
 {
     unsigned ia = static_cast<PropertyMapEntry* const*>(a)[0]->index;

diff --git a/JavaScriptCore/kjs/PropertyMap.h b/JavaScriptCore/kjs/PropertyMap.h
index 186146574695e6a308902f0ab9230ba7208d7a1c..3c18a85bd2546893d560fd82683ef1b7e88de5c4 100644 (file)
--- a/JavaScriptCore/kjs/PropertyMap.h
+++ b/JavaScriptCore/kjs/PropertyMap.h
@@ -91,12 +91,8 @@ namespace JSC {
 
         void put(const Identifier& propertyName, JSValue*, unsigned attributes, bool checkReadOnly, JSObject* slotBase, PutPropertySlot&, PropertyStorage&);
         void remove(const Identifier& propertyName, PropertyStorage&);
-        JSValue* get(const Identifier& propertyName, const PropertyStorage&) const;
-        JSValue* get(const Identifier& propertyName, unsigned& attributes, const PropertyStorage&) const;
-        JSValue** getLocation(const Identifier& propertyName, const PropertyStorage&);
-        JSValue** getLocation(const Identifier& propertyName, bool& isWriteable, const PropertyStorage&);
-
         size_t getOffset(const Identifier& propertyName);
+        size_t getOffset(const Identifier& propertyName, unsigned& attributes);
         size_t getOffset(const Identifier& propertyName, bool& isWriteable);
 
         void getEnumerablePropertyNames(PropertyNameArray&) const;