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)
{