Diff
Modified: trunk/JSTests/ChangeLog (277540 => 277541)
--- trunk/JSTests/ChangeLog 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/JSTests/ChangeLog 2021-05-15 16:59:57 UTC (rev 277541)
@@ -1,3 +1,12 @@
+2021-05-15 Alexey Shvayka <[email protected]>
+
+ Turn callGetter() / callSetter() into instance methods
+ https://bugs.webkit.org/show_bug.cgi?id=225831
+
+ Reviewed by Ross Kirsling.
+
+ * microbenchmarks/put-slow-no-cache-setter.js: Added.
+
2021-05-07 Ross Kirsling <[email protected]>
[JSC] Error#cause must recognize explicit undefined
Added: trunk/JSTests/microbenchmarks/put-slow-no-cache-setter.js (0 => 277541)
--- trunk/JSTests/microbenchmarks/put-slow-no-cache-setter.js (rev 0)
+++ trunk/JSTests/microbenchmarks/put-slow-no-cache-setter.js 2021-05-15 16:59:57 UTC (rev 277541)
@@ -0,0 +1,16 @@
+(function() {
+ var foo = 0;
+ var base = {
+ set foo0(v) { foo = v; },
+ set foo1(v) { foo = v; },
+ set foo2(v) { foo = v; },
+ set foo3(v) { foo = v; },
+ set foo4(v) { foo = v; },
+ };
+
+ for (var j = 0; j < 3e5; j++)
+ base[`foo${j % 5}`] = j;
+
+ if (foo !== j - 1)
+ throw new Error("Bad assertion!");
+})();
Modified: trunk/Source/_javascript_Core/ChangeLog (277540 => 277541)
--- trunk/Source/_javascript_Core/ChangeLog 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-05-15 16:59:57 UTC (rev 277541)
@@ -1,3 +1,41 @@
+2021-05-15 Alexey Shvayka <[email protected]>
+
+ Turn callGetter() / callSetter() into instance methods
+ https://bugs.webkit.org/show_bug.cgi?id=225831
+
+ Reviewed by Ross Kirsling.
+
+ 1. Turn callGetter() / callSetter() into instance methods to simplify their signatures.
+ 2. Rename `base` parameter to `thisValue`, avoiding similarity with slotBase().
+ 3. Accept `bool shouldThrow` in callSetter() since ECMAMode is cumbersome to construct.
+ 4. Replace isSetterNull(), which does LIKELY(inherits), with direct JSType check.
+ 5. Introduce getCallData(VM&, JSCell*) overload to avoid extra checks / casts.
+ 6. Move isValidCallee() to JSCell and handle primitives gracefully.
+
+ No behavior change. Advances provided callSetter() microbenchmark by 2%.
+
+ * runtime/GetterSetter.cpp:
+ (JSC::GetterSetter::callGetter):
+ (JSC::GetterSetter::callSetter):
+ (JSC::callGetter): Deleted.
+ (JSC::callSetter): Deleted.
+ * runtime/GetterSetter.h:
+ * runtime/JSCJSValue.cpp:
+ (JSC::JSValue::isValidCallee): Deleted.
+ * runtime/JSCJSValue.h:
+ * runtime/JSCell.cpp:
+ (JSC::JSCell::isValidCallee const):
+ * runtime/JSCell.h:
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::putInlineSlow):
+ * runtime/JSObjectInlines.h:
+ (JSC::getCallData):
+ (JSC::getConstructData):
+ * runtime/PropertySlot.cpp:
+ (JSC::PropertySlot::functionGetter const):
+ * runtime/SparseArrayValueMap.cpp:
+ (JSC::SparseArrayEntry::put):
+
2021-05-14 Chris Dumez <[email protected]>
Drop FileSystem::fileMetadata() / fileMetadataFollowingSymlinks()
Modified: trunk/Source/_javascript_Core/runtime/GetterSetter.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/GetterSetter.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/GetterSetter.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -46,7 +46,7 @@
DEFINE_VISIT_CHILDREN(GetterSetter);
-JSValue callGetter(JSGlobalObject* globalObject, JSValue base, JSValue getterSetter)
+JSValue GetterSetter::callGetter(JSGlobalObject* globalObject, JSValue thisValue)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -54,31 +54,29 @@
// We work around that by checking here.
RETURN_IF_EXCEPTION(scope, scope.exception()->value());
- JSObject* getter = jsCast<GetterSetter*>(getterSetter)->getter();
+ JSObject* getter = this->getter();
- auto callData = getCallData(vm, getter);
- RELEASE_AND_RETURN(scope, call(globalObject, getter, callData, base, ArgList()));
+ auto callData = JSC::getCallData(vm, getter);
+ RELEASE_AND_RETURN(scope, call(globalObject, getter, callData, thisValue, ArgList()));
}
-bool callSetter(JSGlobalObject* globalObject, JSValue base, JSValue getterSetter, JSValue value, ECMAMode ecmaMode)
+bool GetterSetter::callSetter(JSGlobalObject* globalObject, JSValue thisValue, JSValue value, bool shouldThrow)
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- GetterSetter* getterSetterObj = jsCast<GetterSetter*>(getterSetter);
+ JSObject* setter = this->setter();
- if (getterSetterObj->isSetterNull())
- return typeError(globalObject, scope, ecmaMode.isStrict(), ReadonlyPropertyWriteError);
+ if (setter->type() == NullSetterFunctionType)
+ return typeError(globalObject, scope, shouldThrow, ReadonlyPropertyWriteError);
- JSObject* setter = getterSetterObj->setter();
-
MarkedArgumentBuffer args;
args.append(value);
ASSERT(!args.hasOverflowed());
- auto callData = getCallData(vm, setter);
+ auto callData = JSC::getCallData(vm, setter);
scope.release();
- call(globalObject, setter, callData, base, args);
+ call(globalObject, setter, callData, thisValue, args);
return true;
}
Modified: trunk/Source/_javascript_Core/runtime/GetterSetter.h (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/GetterSetter.h 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/GetterSetter.h 2021-05-15 16:59:57 UTC (rev 277541)
@@ -105,6 +105,9 @@
return result;
}
+ JSValue callGetter(JSGlobalObject*, JSValue thisValue);
+ bool callSetter(JSGlobalObject*, JSValue thisValue, JSValue, bool shouldThrow);
+
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
return Structure::create(vm, globalObject, prototype, TypeInfo(GetterSetterType, StructureFlags), info());
@@ -134,7 +137,4 @@
WriteBarrier<JSObject> m_setter;
};
-JSValue callGetter(JSGlobalObject*, JSValue base, JSValue getterSetter);
-JS_EXPORT_PRIVATE bool callSetter(JSGlobalObject*, JSValue base, JSValue getterSetter, JSValue, ECMAMode);
-
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -384,11 +384,6 @@
out.print("INVALID");
}
-bool JSValue::isValidCallee()
-{
- return asObject(asCell())->globalObject();
-}
-
JSString* JSValue::toStringSlowCase(JSGlobalObject* globalObject, bool returnEmptyStringOnError) const
{
VM& vm = globalObject->vm();
Modified: trunk/Source/_javascript_Core/runtime/JSCJSValue.h (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSCJSValue.h 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSCJSValue.h 2021-05-15 16:59:57 UTC (rev 277541)
@@ -343,7 +343,6 @@
bool isCell() const;
JSCell* asCell() const;
- JS_EXPORT_PRIVATE bool isValidCallee();
Structure* structureOrNull(VM&) const;
Modified: trunk/Source/_javascript_Core/runtime/JSCell.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSCell.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSCell.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -98,6 +98,11 @@
return { };
}
+bool JSCell::isValidCallee() const
+{
+ return isObject() && asObject(this)->globalObject();
+}
+
bool JSCell::put(JSCell* cell, JSGlobalObject* globalObject, PropertyName identifier, JSValue value, PutPropertySlot& slot)
{
if (cell->isString() || cell->isSymbol() || cell->isHeapBigInt())
Modified: trunk/Source/_javascript_Core/runtime/JSCell.h (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSCell.h 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSCell.h 2021-05-15 16:59:57 UTC (rev 277541)
@@ -114,6 +114,7 @@
template<Concurrency> TriState isConstructorWithConcurrency(VM&);
bool inherits(VM&, const ClassInfo*) const;
template<typename Target> bool inherits(VM&) const;
+ JS_EXPORT_PRIVATE bool isValidCallee() const;
bool isAPIValueWrapper() const;
// Each cell has a built-in lock. Currently it's simply available for use if you need it. It's
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -811,8 +811,7 @@
// We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
if (!this->structure(vm)->isUncacheableDictionary())
slot.setCacheableSetter(obj, offset);
- JSValue gs = obj->getDirect(offset);
- RELEASE_AND_RETURN(scope, callSetter(globalObject, slot.thisValue(), gs, value, slot.isStrictMode() ? ECMAMode::strict() : ECMAMode::sloppy()));
+ RELEASE_AND_RETURN(scope, jsCast<GetterSetter*>(obj->getDirect(offset))->callSetter(globalObject, slot.thisValue(), value, slot.isStrictMode()));
}
if (attributes & PropertyAttribute::CustomAccessor) {
// FIXME: Remove this after WebIDL generator is fixed to set ReadOnly for [RuntimeConditionallyReadWrite] attributes.
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-05-15 16:59:57 UTC (rev 277541)
@@ -539,18 +539,22 @@
return putDirectInternal<PutModePut>(vm, propertyName, value, 0, slot);
}
-inline CallData getCallData(VM& vm, JSValue value)
+ALWAYS_INLINE CallData getCallData(VM& vm, JSCell* cell)
{
- if (!value.isCell())
- return { };
- JSCell* cell = value.asCell();
if (cell->type() == JSFunctionType)
return JSFunction::getCallData(cell);
CallData result = cell->methodTable(vm)->getCallData(cell);
- ASSERT(result.type == CallData::Type::None || value.isValidCallee());
+ ASSERT(result.type == CallData::Type::None || cell->isValidCallee());
return result;
}
+inline CallData getCallData(VM& vm, JSValue value)
+{
+ if (!value.isCell())
+ return { };
+ return getCallData(vm, value.asCell());
+}
+
inline CallData getConstructData(VM& vm, JSValue value)
{
if (!value.isCell())
@@ -559,7 +563,7 @@
if (cell->type() == JSFunctionType)
return JSFunction::getConstructData(cell);
CallData result = cell->methodTable(vm)->getConstructData(cell);
- ASSERT(result.type == CallData::Type::None || value.isValidCallee());
+ ASSERT(result.type == CallData::Type::None || cell->isValidCallee());
return result;
}
Modified: trunk/Source/_javascript_Core/runtime/PropertySlot.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/PropertySlot.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -30,7 +30,7 @@
JSValue PropertySlot::functionGetter(JSGlobalObject* globalObject) const
{
ASSERT(m_thisValue);
- return callGetter(globalObject, m_thisValue, m_data.getter.getterSetter);
+ return m_data.getter.getterSetter->callGetter(globalObject, m_thisValue);
}
JSValue PropertySlot::customGetter(JSGlobalObject* globalObject, PropertyName propertyName) const
Modified: trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp (277540 => 277541)
--- trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp 2021-05-15 16:21:39 UTC (rev 277540)
+++ trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp 2021-05-15 16:59:57 UTC (rev 277541)
@@ -201,7 +201,7 @@
return true;
}
- RELEASE_AND_RETURN(scope, callSetter(globalObject, thisValue, Base::get(), value, shouldThrow ? ECMAMode::strict() : ECMAMode::sloppy()));
+ RELEASE_AND_RETURN(scope, jsCast<GetterSetter*>(Base::get())->callSetter(globalObject, thisValue, value, shouldThrow));
}
JSValue SparseArrayEntry::getNonSparseMode() const