Optimize own property GetByVals with rope string subscripts.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Sep 2014 22:29:59 +0000 (22:29 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Sep 2014 22:29:59 +0000 (22:29 +0000)
<https://webkit.org/b/136458>

For simple JSObjects that don't override getOwnPropertySlot to implement
custom properties, we have a fast path that grabs directly at the object
property storage.

Make this fast path even faster when the property name is an unresolved
rope string by using JSString::toExistingAtomicString(). This is faster
because it avoids allocating a new StringImpl if the string is already
a known Identifier, which is guaranteed to be the case if it's present
as an own property on the object.)

~10% speed-up on Dromaeo/dom-attr.html

Reviewed by Geoffrey Garen.

* dfg/DFGOperations.cpp:
* jit/JITOperations.cpp:
(JSC::getByVal):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::getByVal):

    When using the fastGetOwnProperty() optimization, get the String
    out of JSString by using toExistingAtomicString(). This avoids
    StringImpl allocation and lets us bypass the PropertyTable lookup
    entirely if no AtomicString is found.

* runtime/JSCell.h:
* runtime/JSCellInlines.h:
(JSC::JSCell::fastGetOwnProperty):

    Make fastGetOwnProperty() take a PropertyName instead of a String.
    This avoids churning the ref count, since we don't need to create
    a temporary wrapper around the AtomicStringImpl* found in GetByVal.

* runtime/PropertyName.h:
(JSC::PropertyName::PropertyName):

    Add constructor: PropertyName(AtomicStringImpl*)

* runtime/PropertyMapHashTable.h:
(JSC::PropertyTable::get):
(JSC::PropertyTable::findWithString): Deleted.
* runtime/Structure.h:
* runtime/StructureInlines.h:
(JSC::Structure::get):

    Remove code for querying a PropertyTable with an unhashed string key
    since the only client is now gone.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Source/JavaScriptCore/runtime/JSCell.h
Source/JavaScriptCore/runtime/JSCellInlines.h
Source/JavaScriptCore/runtime/PropertyMapHashTable.h
Source/JavaScriptCore/runtime/PropertyName.h
Source/JavaScriptCore/runtime/Structure.h
Source/JavaScriptCore/runtime/StructureInlines.h

index 33f7d6c..e35189e 100644 (file)
@@ -1,3 +1,56 @@
+2014-09-02  Andreas Kling  <akling@apple.com>
+
+        Optimize own property GetByVals with rope string subscripts.
+        <https://webkit.org/b/136458>
+
+        For simple JSObjects that don't override getOwnPropertySlot to implement
+        custom properties, we have a fast path that grabs directly at the object
+        property storage.
+
+        Make this fast path even faster when the property name is an unresolved
+        rope string by using JSString::toExistingAtomicString(). This is faster
+        because it avoids allocating a new StringImpl if the string is already
+        a known Identifier, which is guaranteed to be the case if it's present
+        as an own property on the object.)
+
+        ~10% speed-up on Dromaeo/dom-attr.html
+
+        Reviewed by Geoffrey Garen.
+
+        * dfg/DFGOperations.cpp:
+        * jit/JITOperations.cpp:
+        (JSC::getByVal):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::getByVal):
+
+            When using the fastGetOwnProperty() optimization, get the String
+            out of JSString by using toExistingAtomicString(). This avoids
+            StringImpl allocation and lets us bypass the PropertyTable lookup
+            entirely if no AtomicString is found.
+
+        * runtime/JSCell.h:
+        * runtime/JSCellInlines.h:
+        (JSC::JSCell::fastGetOwnProperty):
+
+            Make fastGetOwnProperty() take a PropertyName instead of a String.
+            This avoids churning the ref count, since we don't need to create
+            a temporary wrapper around the AtomicStringImpl* found in GetByVal.
+
+        * runtime/PropertyName.h:
+        (JSC::PropertyName::PropertyName):
+
+            Add constructor: PropertyName(AtomicStringImpl*)
+
+        * runtime/PropertyMapHashTable.h:
+        (JSC::PropertyTable::get):
+        (JSC::PropertyTable::findWithString): Deleted.
+        * runtime/Structure.h:
+        * runtime/StructureInlines.h:
+        (JSC::Structure::get):
+
+            Remove code for querying a PropertyTable with an unhashed string key
+            since the only client is now gone.
+
 2014-09-02  Dániel Bátyai  <dbatyai.u-szeged@partner.samsung.com>
 
         [ARM] MacroAssembler generating incorrect code on ARM32 Traditional
index 07847e8..4d9843c 100644 (file)
@@ -297,8 +297,10 @@ EncodedJSValue JIT_OPERATION operationGetByVal(ExecState* exec, EncodedJSValue e
         } else if (property.isString()) {
             Structure& structure = *base->structure(vm);
             if (JSCell::canUseFastGetOwnProperty(structure)) {
-                if (JSValue result = base->fastGetOwnProperty(vm, structure, asString(property)->value(exec)))
-                    return JSValue::encode(result);
+                if (AtomicStringImpl* existingAtomicString = asString(property)->toExistingAtomicString(exec)) {
+                    if (JSValue result = base->fastGetOwnProperty(vm, structure, existingAtomicString))
+                        return JSValue::encode(result);
+                }
             }
         }
     }
@@ -327,8 +329,10 @@ EncodedJSValue JIT_OPERATION operationGetByValCell(ExecState* exec, JSCell* base
     } else if (property.isString()) {
         Structure& structure = *base->structure(vm);
         if (JSCell::canUseFastGetOwnProperty(structure)) {
-            if (JSValue result = base->fastGetOwnProperty(vm, structure, asString(property)->value(exec)))
-                return JSValue::encode(result);
+            if (AtomicStringImpl* existingAtomicString = asString(property)->toExistingAtomicString(exec)) {
+                if (JSValue result = base->fastGetOwnProperty(vm, structure, existingAtomicString))
+                    return JSValue::encode(result);
+            }
         }
     }
 
index 49fa36b..7c2b024 100644 (file)
@@ -1423,8 +1423,10 @@ static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, R
         VM& vm = exec->vm();
         Structure& structure = *baseValue.asCell()->structure(vm);
         if (JSCell::canUseFastGetOwnProperty(structure)) {
-            if (JSValue result = baseValue.asCell()->fastGetOwnProperty(vm, structure, asString(subscript)->value(exec)))
-                return result;
+            if (AtomicStringImpl* existingAtomicString = asString(subscript)->toExistingAtomicString(exec)) {
+                if (JSValue result = baseValue.asCell()->fastGetOwnProperty(vm, structure, existingAtomicString))
+                    return result;
+            }
         }
     }
 
index 949f61f..2f730f7 100644 (file)
@@ -725,8 +725,10 @@ inline JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript)
         VM& vm = exec->vm();
         Structure& structure = *baseValue.asCell()->structure(vm);
         if (JSCell::canUseFastGetOwnProperty(structure)) {
-            if (JSValue result = baseValue.asCell()->fastGetOwnProperty(vm, structure, asString(subscript)->value(exec)))
-                return result;
+            if (AtomicStringImpl* existingAtomicString = asString(subscript)->toExistingAtomicString(exec)) {
+                if (JSValue result = baseValue.asCell()->fastGetOwnProperty(vm, structure, existingAtomicString))
+                    return result;
+            }
         }
     }
     
index 52d39e0..c6037fb 100644 (file)
@@ -39,6 +39,7 @@ namespace JSC {
 
 class CopyVisitor;
 class ExecState;
+class Identifier;
 class JSArrayBufferView;
 class JSDestructibleObject;
 class JSGlobalObject;
@@ -141,7 +142,7 @@ public:
     bool isZapped() const { return !*reinterpret_cast<uintptr_t* const*>(this); }
 
     static bool canUseFastGetOwnProperty(const Structure&);
-    JSValue fastGetOwnProperty(VM&, Structure&, const String&);
+    JSValue fastGetOwnProperty(VM&, Structure&, PropertyName);
 
     enum GCData : uint8_t {
         Marked = 0,
index 85a431e..8a98c29 100644 (file)
@@ -209,16 +209,10 @@ inline bool JSCell::inherits(const ClassInfo* info) const
     return classInfo()->isSubClassOf(info);
 }
 
-// Fast call to get a property where we may not yet have converted the string to an
-// identifier. The first time we perform a property access with a given string, try
-// performing the property map lookup without forming an identifier. We detect this
-// case by checking whether the hash has yet been set for this string.
-ALWAYS_INLINE JSValue JSCell::fastGetOwnProperty(VM& vm, Structure& structure, const String& name)
+ALWAYS_INLINE JSValue JSCell::fastGetOwnProperty(VM& vm, Structure& structure, PropertyName name)
 {
     ASSERT(canUseFastGetOwnProperty(structure));
-    PropertyOffset offset = name.impl()->hasHash()
-        ? structure.get(vm, Identifier(&vm, name))
-        : structure.get(vm, name);
+    PropertyOffset offset = structure.get(vm, name);
     if (offset != invalidOffset)
         return asObject(this)->locationForOffset(offset)->get();
     return JSValue();
index 9082468..1d652de 100644 (file)
@@ -171,7 +171,6 @@ public:
 
     // Find a value in the table.
     find_iterator find(const KeyType&);
-    find_iterator findWithString(const KeyType&);
     ValueType* get(const KeyType&);
     // Add a value to the table
     enum EffectOnPropertyOffset { PropertyOffsetMayChange, PropertyOffsetMustNotChange };
@@ -349,35 +348,6 @@ inline PropertyTable::ValueType* PropertyTable::get(const KeyType& key)
     }
 }
 
-inline PropertyTable::find_iterator PropertyTable::findWithString(const KeyType& key)
-{
-    ASSERT(key);
-    ASSERT(!key->isAtomic() && !key->hasHash());
-    unsigned hash = key->hash();
-    unsigned step = 0;
-
-#if DUMP_PROPERTYMAP_STATS
-    ++propertyMapHashTableStats->numLookups;
-#endif
-
-    while (true) {
-        unsigned entryIndex = m_index[hash & m_indexMask];
-        if (entryIndex == EmptyEntryIndex)
-            return std::make_pair((ValueType*)0, hash & m_indexMask);
-        const KeyType& keyInMap = table()[entryIndex - 1].key;
-        if (equal(key, keyInMap) && keyInMap->isAtomic())
-            return std::make_pair(&table()[entryIndex - 1], hash & m_indexMask);
-
-#if DUMP_PROPERTYMAP_STATS
-        ++propertyMapHashTableStats->numLookupProbing;
-#endif
-
-        if (!step)
-            step = WTF::doubleHash(key->existingHash()) | 1;
-        hash += step;
-    }
-}
-
 inline std::pair<PropertyTable::find_iterator, bool> PropertyTable::add(const ValueType& entry, PropertyOffset& offset, EffectOnPropertyOffset offsetEffect)
 {
     // Look for a value with a matching key already in the array.
index 9e99f53..d739cef 100644 (file)
@@ -78,12 +78,17 @@ ALWAYS_INLINE uint32_t toUInt32FromStringImpl(StringImpl* impl)
 
 class PropertyName {
 public:
-    PropertyName(const Identifier& propertyName)
-        : m_impl(static_cast<AtomicStringImpl*>(propertyName.impl()))
+    PropertyName(AtomicStringImpl* propertyName)
+        : m_impl(propertyName)
     {
         ASSERT(!m_impl || m_impl->isAtomic());
     }
 
+    PropertyName(const Identifier& propertyName)
+        : PropertyName(static_cast<AtomicStringImpl*>(propertyName.impl()))
+    {
+    }
+
     PropertyName(const PrivateName& propertyName)
         : m_impl(static_cast<AtomicStringImpl*>(propertyName.uid()))
     {
index fc9878c..94e6633 100644 (file)
@@ -260,7 +260,6 @@ public:
     bool masqueradesAsUndefined(JSGlobalObject* lexicalGlobalObject);
 
     PropertyOffset get(VM&, PropertyName);
-    PropertyOffset get(VM&, const WTF::String& name);
     PropertyOffset get(VM&, PropertyName, unsigned& attributes);
 
     PropertyOffset getConcurrently(VM&, StringImpl* uid);
index ffb7577..9820989 100644 (file)
@@ -85,19 +85,6 @@ ALWAYS_INLINE PropertyOffset Structure::get(VM& vm, PropertyName propertyName)
     PropertyMapEntry* entry = propertyTable->get(propertyName.uid());
     return entry ? entry->offset : invalidOffset;
 }
-
-ALWAYS_INLINE PropertyOffset Structure::get(VM& vm, const WTF::String& name)
-{
-    ASSERT(!isCompilationThread());
-    ASSERT(structure()->classInfo() == info());
-    PropertyTable* propertyTable;
-    materializePropertyMapIfNecessary(vm, propertyTable);
-    if (!propertyTable)
-        return invalidOffset;
-
-    PropertyMapEntry* entry = propertyTable->findWithString(name.impl()).first;
-    return entry ? entry->offset : invalidOffset;
-}
     
 ALWAYS_INLINE PropertyOffset Structure::get(VM& vm, PropertyName propertyName, unsigned& attributes)
 {