Title: [173188] trunk/Source/_javascript_Core
Revision
173188
Author
[email protected]
Date
2014-09-02 15:29:59 -0700 (Tue, 02 Sep 2014)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (173187 => 173188)


--- trunk/Source/_javascript_Core/ChangeLog	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-09-02 22:29:59 UTC (rev 173188)
@@ -1,3 +1,56 @@
+2014-09-02  Andreas Kling  <[email protected]>
+
+        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  <[email protected]>
 
         [ARM] MacroAssembler generating incorrect code on ARM32 Traditional

Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (173187 => 173188)


--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2014-09-02 22:29:59 UTC (rev 173188)
@@ -297,8 +297,10 @@
         } 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 @@
     } 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);
+            }
         }
     }
 

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (173187 => 173188)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2014-09-02 22:29:59 UTC (rev 173188)
@@ -1423,8 +1423,10 @@
         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;
+            }
         }
     }
 

Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (173187 => 173188)


--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2014-09-02 22:29:59 UTC (rev 173188)
@@ -725,8 +725,10 @@
         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;
+            }
         }
     }
     

Modified: trunk/Source/_javascript_Core/runtime/JSCell.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/JSCell.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/JSCell.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -39,6 +39,7 @@
 
 class CopyVisitor;
 class ExecState;
+class Identifier;
 class JSArrayBufferView;
 class JSDestructibleObject;
 class JSGlobalObject;
@@ -141,7 +142,7 @@
     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,

Modified: trunk/Source/_javascript_Core/runtime/JSCellInlines.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/JSCellInlines.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/JSCellInlines.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -209,16 +209,10 @@
     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();

Modified: trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/PropertyMapHashTable.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -171,7 +171,6 @@
 
     // 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::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.

Modified: trunk/Source/_javascript_Core/runtime/PropertyName.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/PropertyName.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/PropertyName.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -78,12 +78,17 @@
 
 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()))
     {

Modified: trunk/Source/_javascript_Core/runtime/Structure.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/Structure.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/Structure.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -260,7 +260,6 @@
     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);

Modified: trunk/Source/_javascript_Core/runtime/StructureInlines.h (173187 => 173188)


--- trunk/Source/_javascript_Core/runtime/StructureInlines.h	2014-09-02 21:33:41 UTC (rev 173187)
+++ trunk/Source/_javascript_Core/runtime/StructureInlines.h	2014-09-02 22:29:59 UTC (rev 173188)
@@ -85,19 +85,6 @@
     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)
 {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to