Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (168334 => 168335)
--- trunk/Source/_javascript_Core/ChangeLog 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-05-06 00:53:29 UTC (rev 168335)
@@ -1,5 +1,31 @@
2014-05-05 Andreas Kling <[email protected]>
+ Optimize GetByVal when subscript is a rope string.
+ <https://webkit.org/b/132590>
+
+ Use JSString::toIdentifier() in the various GetByVal implementations
+ to try and avoid allocating extra strings.
+
+ Added canUseFastGetOwnProperty() and wrap calls to fastGetOwnProperty()
+ in that, to avoid calling JSString::value() which always resolves ropes
+ into new strings and de-optimizes subsequent toIdentifier() calls.
+
+ My iMac says ~9% progression on Dromaeo/dom-attr.html
+
+ Reviewed by Phil Pizlo.
+
+ * dfg/DFGOperations.cpp:
+ * jit/JITOperations.cpp:
+ (JSC::getByVal):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::getByVal):
+ * runtime/JSCell.h:
+ * runtime/JSCellInlines.h:
+ (JSC::JSCell::fastGetOwnProperty):
+ (JSC::JSCell::canUseFastGetOwnProperty):
+
+2014-05-05 Andreas Kling <[email protected]>
+
REGRESSION (r168256): ASSERTION FAILED: (buffer + m_length) == position loading vanityfair.com article.
<https://webkit.org/b/168256>
<rdar://problem/16816316>
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (168334 => 168335)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2014-05-06 00:53:29 UTC (rev 168335)
@@ -295,15 +295,18 @@
if (propertyAsUInt32 == propertyAsDouble)
return getByVal(exec, base, propertyAsUInt32);
} else if (property.isString()) {
- if (JSValue result = base->fastGetOwnProperty(vm, asString(property)->value(exec)))
- return JSValue::encode(result);
+ 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 (isName(property))
return JSValue::encode(baseValue.get(exec, jsCast<NameInstance*>(property.asCell())->privateName()));
- Identifier ident(exec, property.toString(exec)->value(exec));
+ Identifier ident = property.toString(exec)->toIdentifier(exec);
return JSValue::encode(baseValue.get(exec, ident));
}
@@ -322,14 +325,17 @@
if (propertyAsUInt32 == propertyAsDouble)
return getByVal(exec, base, propertyAsUInt32);
} else if (property.isString()) {
- if (JSValue result = base->fastGetOwnProperty(vm, asString(property)->value(exec)))
- return JSValue::encode(result);
+ 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 (isName(property))
return JSValue::encode(JSValue(base).get(exec, jsCast<NameInstance*>(property.asCell())->privateName()));
- Identifier ident(exec, property.toString(exec)->value(exec));
+ Identifier ident = property.toString(exec)->toIdentifier(exec);
return JSValue::encode(JSValue(base).get(exec, ident));
}
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (168334 => 168335)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-05-06 00:53:29 UTC (rev 168335)
@@ -1420,8 +1420,12 @@
static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, ReturnAddressPtr returnAddress)
{
if (LIKELY(baseValue.isCell() && subscript.isString())) {
- if (JSValue result = baseValue.asCell()->fastGetOwnProperty(exec->vm(), asString(subscript)->value(exec)))
- return result;
+ 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 (subscript.isUInt32()) {
@@ -1436,7 +1440,7 @@
if (isName(subscript))
return baseValue.get(exec, jsCast<NameInstance*>(subscript.asCell())->privateName());
- Identifier property(exec, subscript.toString(exec)->value(exec));
+ Identifier property = subscript.toString(exec)->toIdentifier(exec);
return baseValue.get(exec, property);
}
@@ -1518,7 +1522,7 @@
} else if (isName(subscript))
result = baseValue.get(exec, jsCast<NameInstance*>(subscript.asCell())->privateName());
else {
- Identifier property(exec, subscript.toString(exec)->value(exec));
+ Identifier property = subscript.toString(exec)->toIdentifier(exec);
result = baseValue.get(exec, property);
}
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (168334 => 168335)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-05-06 00:53:29 UTC (rev 168335)
@@ -715,8 +715,12 @@
inline JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript)
{
if (LIKELY(baseValue.isCell() && subscript.isString())) {
- if (JSValue result = baseValue.asCell()->fastGetOwnProperty(exec->vm(), asString(subscript)->value(exec)))
- return result;
+ 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 (subscript.isUInt32()) {
@@ -730,7 +734,7 @@
if (isName(subscript))
return baseValue.get(exec, jsCast<NameInstance*>(subscript.asCell())->privateName());
- Identifier property(exec, subscript.toString(exec)->value(exec));
+ Identifier property = subscript.toString(exec)->toIdentifier(exec);
return baseValue.get(exec, property);
}
Modified: trunk/Source/_javascript_Core/runtime/JSCell.h (168334 => 168335)
--- trunk/Source/_javascript_Core/runtime/JSCell.h 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/runtime/JSCell.h 2014-05-06 00:53:29 UTC (rev 168335)
@@ -144,7 +144,8 @@
void zap() { *reinterpret_cast<uintptr_t**>(this) = 0; }
bool isZapped() const { return !*reinterpret_cast<uintptr_t* const*>(this); }
- JSValue fastGetOwnProperty(VM&, const String&);
+ static bool canUseFastGetOwnProperty(const Structure&);
+ JSValue fastGetOwnProperty(VM&, Structure&, const String&);
enum GCData : uint8_t {
Marked = 0,
Modified: trunk/Source/_javascript_Core/runtime/JSCellInlines.h (168334 => 168335)
--- trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2014-05-06 00:47:33 UTC (rev 168334)
+++ trunk/Source/_javascript_Core/runtime/JSCellInlines.h 2014-05-06 00:53:29 UTC (rev 168335)
@@ -213,19 +213,22 @@
// 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, const String& name)
+ALWAYS_INLINE JSValue JSCell::fastGetOwnProperty(VM& vm, Structure& structure, const String& name)
{
- Structure& structure = *this->structure(vm);
- if (!structure.typeInfo().overridesGetOwnPropertySlot() && !structure.hasGetterSetterProperties()) {
- PropertyOffset offset = name.impl()->hasHash()
- ? structure.get(vm, Identifier(&vm, name))
- : structure.get(vm, name);
- if (offset != invalidOffset)
- return asObject(this)->locationForOffset(offset)->get();
- }
+ ASSERT(canUseFastGetOwnProperty(vm));
+ PropertyOffset offset = name.impl()->hasHash()
+ ? structure.get(vm, Identifier(&vm, name))
+ : structure.get(vm, name);
+ if (offset != invalidOffset)
+ return asObject(this)->locationForOffset(offset)->get();
return JSValue();
}
+inline bool JSCell::canUseFastGetOwnProperty(const Structure& structure)
+{
+ return !structure.hasGetterSetterProperties() && !structure.typeInfo().overridesGetOwnPropertySlot();
+}
+
inline bool JSCell::toBoolean(ExecState* exec) const
{
if (isString())
