Reviewed by Eric.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 28 Oct 2007 09:20:48 +0000 (09:20 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 28 Oct 2007 09:20:48 +0000 (09:20 +0000)
        - switch SymbolTable to be a HashMap instead of a PropertyMap for 3% SunSpider speedup

        * kjs/SymbolTable.h:
        (KJS::IdentifierRepHash::hash): Special hash function for identifier reps.
        (KJS::IdentifierRepHash::equal): ditto
        (KJS::SymbolTableIndexHashTraits::emptyValue): Special HashTraits for the index value.
        (KJS::SymbolTable): change to a typedef for a HashMap.
        * kjs/function.cpp:
        (KJS::ActivationImp::getOwnPropertySlot): Adjusted for new SymbolTable API.
        (KJS::ActivationImp::deleteProperty): ditto
        (KJS::ActivationImp::put): ditto

        * kjs/nodes.cpp:
        (KJS::FunctionBodyNode::initializesymbolTable): Adjusted, since
        you now have to store a UString::rep, not an identifier.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/SymbolTable.h
JavaScriptCore/kjs/function.cpp
JavaScriptCore/kjs/nodes.cpp

index 4ba07b3..103bcd1 100644 (file)
@@ -1,3 +1,23 @@
+2007-10-28  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Eric.
+        
+        - switch SymbolTable to be a HashMap instead of a PropertyMap for 3% SunSpider speedup
+
+        * kjs/SymbolTable.h:
+        (KJS::IdentifierRepHash::hash): Special hash function for identifier reps.
+        (KJS::IdentifierRepHash::equal): ditto
+        (KJS::SymbolTableIndexHashTraits::emptyValue): Special HashTraits for the index value.
+        (KJS::SymbolTable): change to a typedef for a HashMap.
+        * kjs/function.cpp:
+        (KJS::ActivationImp::getOwnPropertySlot): Adjusted for new SymbolTable API.
+        (KJS::ActivationImp::deleteProperty): ditto
+        (KJS::ActivationImp::put): ditto
+
+        * kjs/nodes.cpp:
+        (KJS::FunctionBodyNode::initializesymbolTable): Adjusted, since
+        you now have to store a UString::rep, not an identifier.
+
 2007-10-27  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Oliver.
index 74fc805..fb7df2a 100644 (file)
@@ -35,28 +35,23 @@ namespace KJS {
 
     class JSValue;
 
-    // SymbolTable is implemented in terms of a PropertyMap hack for now,
-    // but it should move to WTF::HashMap once that's fast enough.
-
-    class SymbolTable : private PropertyMap {
-    public:
-        void set(const Identifier& name, size_t index)
-        {
-            JSValue* v = reinterpret_cast<JSValue*>(index + 1); // Increment index by 1 because PropertyMap uses 0 to mean "not found."
-            ASSERT(v); // Check for overflow.
-            PropertyMap::put(name, v, 0, false);
-        }
-
-        bool get(const Identifier& name, size_t& index)
-        {
-            if (JSValue* v = PropertyMap::get(name)) {
-                index = reinterpret_cast<uintptr_t>(v) - 1; // Decrement index by 1 to balance the increment in set.
-                return true;
-            }
-            return false;
-        }
+    struct IdentifierRepHash {
+        static unsigned hash(const KJS::UString::Rep *key) { return key->computedHash(); }
+        static bool equal(const KJS::UString::Rep *a, const KJS::UString::Rep *b) { return a == b; }
+        static const bool safeToCompareToEmptyOrDeleted = true;
     };
 
+    struct SymbolTableIndexHashTraits {
+        typedef size_t TraitType;
+        typedef SymbolTableIndexHashTraits StorageTraits;
+        static size_t emptyValue() { return SIZE_T_MAX; }
+        static const bool emptyValueIsZero = false;
+        static const bool needsDestruction = false;
+        static const bool needsRef = false;
+    };
+
+    typedef HashMap<UString::Rep*, size_t, IdentifierRepHash, HashTraits<UString::Rep*>, SymbolTableIndexHashTraits> SymbolTable;
+
 } // namespace KJS
 
 #endif // SymbolTable_h
index e03f585..23ccde3 100644 (file)
@@ -420,8 +420,10 @@ bool ActivationImp::getOwnPropertySlot(ExecState* exec, const Identifier& proper
     // activation object.
     ASSERT(!_prop.hasGetterSetterProperties());
 
-    size_t index;
-    if (symbolTable->get(propertyName, index)) {
+    // it's more efficient to just get and check for a special empty
+    // value of SIZE_T_MAX than to do a separate contains check
+    size_t index = symbolTable->get(propertyName.ustring().rep());
+    if (index != SIZE_T_MAX) {
         slot.setValueSlot(this, &d->localStorage[index].value);
         return true;
     }
@@ -445,8 +447,7 @@ bool ActivationImp::deleteProperty(ExecState* exec, const Identifier& propertyNa
     if (propertyName == exec->propertyNames().arguments)
         return false;
 
-    size_t index;
-    if (symbolTable->get(propertyName, index))
+    if (symbolTable->contains(propertyName.ustring().rep()))
         return false;
 
     return JSObject::deleteProperty(exec, propertyName);
@@ -458,8 +459,10 @@ void ActivationImp::put(ExecState*, const Identifier& propertyName, JSValue* val
   ASSERT(!_prop.hasGetterSetterProperties());
   ASSERT(prototype() == jsNull());
 
-  size_t index;
-  if (symbolTable->get(propertyName, index)) {
+  // it's more efficient to just get and check for a special empty
+  // value of SIZE_T_MAX than to do a separate contains check
+  size_t index = symbolTable->get(propertyName.ustring().rep());
+  if (index != SIZE_T_MAX) {
     LocalStorageEntry& entry = d->localStorage[index];
     entry.value = value;
     entry.attributes = attr;
index e4878dc..9301d17 100644 (file)
@@ -2614,13 +2614,13 @@ void FunctionBodyNode::initializesymbolTable()
 
     // The order of additions here implicitly enforces the mutual exclusion described in ECMA 10.1.3.
     for (i = 0, size = m_varStack.size(); i < size; ++i)
-        m_symbolTable.set(m_varStack[i]->ident, count++);
+        m_symbolTable.set(m_varStack[i]->ident.ustring().rep(), count++);
 
     for (i = 0, size = m_parameters.size(); i < size; ++i)
-        m_symbolTable.set(m_parameters[i], count++);
+        m_symbolTable.set(m_parameters[i].ustring().rep(), count++);
 
     for (i = 0, size = m_functionStack.size(); i < size; ++i)
-        m_symbolTable.set(m_functionStack[i]->ident, count++);
+        m_symbolTable.set(m_functionStack[i]->ident.ustring().rep(), count++);
 
     m_initializedSymbolTable = true;
 }