Title: [277541] trunk
Revision
277541
Author
[email protected]
Date
2021-05-15 09:59:57 -0700 (Sat, 15 May 2021)

Log Message

Turn callGetter() / callSetter() into instance methods
https://bugs.webkit.org/show_bug.cgi?id=225831

Reviewed by Ross Kirsling.

JSTests:

* microbenchmarks/put-slow-no-cache-setter.js: Added.

Source/_javascript_Core:

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

Modified Paths

Added Paths

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
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to