[JSC] Optimize ProxyObject's "ownKeys" trap

https://bugs.webkit.org/show_bug.cgi?id=257346 <rdar://problem/109850846> Reviewed by Yusuke Suzuki. It is a common JS pattern to mass-assign properties and clone / merge objects via Object.assign(), and since in Vue.js v3 front-end developers mostly operate on reactive objects (that are wrapped in a ProxyObject), optimizing its "ownKeys" trap and related operations is crucial. This change: 1. Enables [[ProxyHandler]] trap caching for "ownKeys" and "getOwnPropertyDescriptor" traps. 2. Introduces forwardsGetOwnPropertyNamesToTarget() fast path that expands existing property names caching (based on StructureRareData) to handle ProxyObjects which are missing "ownKeys" and "getOwnPropertyDescriptor" traps. This was made possible by [[ProxyHandler]] trap caching added in https://webkit.org/b/256554. 3. Leveraging Structure flags introduced in https://webkit.org/b/255661, skips "ownKeys" trap result validation in the common case of [[ProxyTarget]] being an ordinary extensible object without non-configurable properties. This results in nice speed-up given the spec requires [1] to enumerate all [[ProxyTarget]] properties and ensure that all non-configurable ones are listed by the userland trap code. [1]: https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys (step 16) ToT patch proxy-ownkeys-via-reflect-ownkeys 81.7443+-0.7848 ^ 46.7884+-0.4395 ^ definitely 1.7471x faster proxy-ownkeys-via-reflect-ownkeys-miss-handler 31.4378+-0.4010 ^ 2.2589+-0.0293 ^ definitely 13.9172x faster proxy-ownkeys-via-object-keys 215.2480+-1.1130 ^ 95.5261+-0.7659 ^ definitely 2.2533x faster proxy-ownkeys-via-object-keys-miss-handler 168.9327+-0.7422 ^ 2.2765+-0.0276 ^ definitely 74.2064x faster proxy-ownkeys-via-object-assign 233.9777+-2.1404 ^ 112.9695+-0.8461 ^ definitely 2.0712x faster proxy-ownkeys-via-object-assign-miss-handler 187.8766+-1.0055 ^ 93.1520+-0.1716 ^ definitely 2.0169x faster <geometric> 125.4813+-0.2685 ^ 24.8795+-0.1193 ^ definitely 5.0436x faster * JSTests/microbenchmarks/proxy-ownkeys-via-object-assign-miss-handler.js: Added. * JSTests/microbenchmarks/proxy-ownkeys-via-object-assign.js: Added. * JSTests/microbenchmarks/proxy-ownkeys-via-object-keys-miss-handler.js: Added. * JSTests/microbenchmarks/proxy-ownkeys-via-object-keys.js: Added. * JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys-miss-handler.js: Added. * JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys.js: Added. * Source/JavaScriptCore/runtime/CommonIdentifiers.h: * Source/JavaScriptCore/runtime/ObjectConstructor.cpp: (JSC::ownPropertyKeys): * Source/JavaScriptCore/runtime/ProxyObject.cpp: (JSC::ProxyObject::getHandlerTrap): (JSC::ProxyObject::performInternalMethodGetOwnProperty): (JSC::ProxyObject::forwardsGetOwnPropertyNamesToTarget): (JSC::ProxyObject::performGetOwnPropertyNames): * Source/JavaScriptCore/runtime/ProxyObject.h: (JSC::ProxyObject::isHandlerTrapsCacheValid): Canonical link: https://commits.webkit.org/265218@main
WebKit · Jun 15, 2023 · ef0e6f6 · ef0e6f6
1 parent 3dc8355
commit ef0e6f6
Show file tree

Hide file tree

Showing 10 changed files with 131 additions and 8 deletions.
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-object-assign-miss-handler.js b/JSTests/microbenchmarks/proxy-ownkeys-via-object-assign-miss-handler.js
@@ -0,0 +1,12 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Object.keys(target);
+    var proxy = new Proxy(target, {});
+
+    var lastObj = {};
+    for (var i = 0; i < 2e5; ++i)
+        lastObj = Object.assign({}, proxy);
+
+    if (Object.keys(lastObj).join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-object-assign.js b/JSTests/microbenchmarks/proxy-ownkeys-via-object-assign.js
@@ -0,0 +1,16 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Object.keys(target);
+    var proxy = new Proxy(target, {
+        ownKeys() {
+            return targetKeys;
+        },
+    });
+
+    var lastObj = {};
+    for (var i = 0; i < 2e5; ++i)
+        lastObj = Object.assign({}, proxy);
+
+    if (Object.keys(lastObj).join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-object-keys-miss-handler.js b/JSTests/microbenchmarks/proxy-ownkeys-via-object-keys-miss-handler.js
@@ -0,0 +1,12 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Object.keys(target);
+    var proxy = new Proxy(target, {});
+
+    var proxyKeys;
+    for (var i = 0; i < 2e5; ++i)
+        proxyKeys = Object.keys(proxy);
+
+    if (proxyKeys.join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-object-keys.js b/JSTests/microbenchmarks/proxy-ownkeys-via-object-keys.js
@@ -0,0 +1,16 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Object.keys(target);
+    var proxy = new Proxy(target, {
+        ownKeys() {
+            return targetKeys;
+        },
+    });
+
+    var proxyKeys;
+    for (var i = 0; i < 2e5; ++i)
+        proxyKeys = Object.keys(proxy);
+
+    if (proxyKeys.join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys-miss-handler.js b/JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys-miss-handler.js
@@ -0,0 +1,12 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Reflect.ownKeys(target);
+    var proxy = new Proxy(target, {});
+
+    var proxyKeys;
+    for (var i = 0; i < 2e5; ++i)
+        proxyKeys = Reflect.ownKeys(proxy);
+
+    if (proxyKeys.join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys.js b/JSTests/microbenchmarks/proxy-ownkeys-via-reflect-ownkeys.js
@@ -0,0 +1,16 @@
+(function() {
+    var target = {k0: 0, k1: 1, k2: 2, k3: 3, k4: 4, k5: 5, k6: 6, k7: 7, k8: 8, k9: 9};
+    var targetKeys = Reflect.ownKeys(target);
+    var proxy = new Proxy(target, {
+        ownKeys() {
+            return targetKeys;
+        },
+    });
+
+    var proxyKeys;
+    for (var i = 0; i < 2e5; ++i)
+        proxyKeys = Reflect.ownKeys(proxy);
+
+    if (proxyKeys.join() !== targetKeys.join())
+        throw new Error("Bad assertion!");
+})();
diff --git a/Source/JavaScriptCore/runtime/CommonIdentifiers.h b/Source/JavaScriptCore/runtime/CommonIdentifiers.h
@@ -133,6 +133,7 @@
     macro(from) \
     macro(fromCharCode) \
     macro(get) \
+    macro(getOwnPropertyDescriptor) \
     macro(global) \
     macro(go) \
     macro(granularity) \
@@ -222,6 +223,7 @@
     macro(osrExitSites) \
     macro(osrExits) \
     macro(overflow) \
+    macro(ownKeys) \
     macro(parse) \
     macro(parseInt) \
     macro(parseFloat) \

diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
@@ -1044,6 +1044,12 @@ JSArray* ownPropertyKeys(JSGlobalObject* globalObject, JSObject* object, Propert
 
     auto kind = inferCachedPropertyNamesKind(propertyNameMode, dontEnumPropertiesMode);
 
+    if (object->inherits<ProxyObject>()) {
+        ProxyObject* proxy = jsCast<ProxyObject*>(object);
+        if (proxy->forwardsGetOwnPropertyNamesToTarget(dontEnumPropertiesMode))
+            object = proxy->target();
+    }
+
     // We attempt to look up own property keys cache in Object.keys / Object.getOwnPropertyNames cases.
     if (LIKELY(!globalObject->isHavingABadTime())) {
         if (auto* immutableButterfly = object->structure()->cachedPropertyNames(kind)) {

diff --git a/Source/JavaScriptCore/runtime/ProxyObject.cpp b/Source/JavaScriptCore/runtime/ProxyObject.cpp
@@ -104,9 +104,7 @@ JSObject* ProxyObject::getHandlerTrap(JSGlobalObject* globalObject, JSObject* ha
         return asObject(value);
     };
 
-    JSValue handlerPrototype = handler->getPrototypeDirect();
-    bool isCacheValid = handler->structureID() == m_handlerStructureID.value() && asObject(handlerPrototype)->structureID() == m_handlerPrototypeStructureID.value();
-    if (isCacheValid) {
+    if (isHandlerTrapsCacheValid(handler)) {
         PropertyOffset offset = m_handlerTrapsOffsetsCache[static_cast<uint8_t>(trap)];
         if (offset == invalidOffset)
             return nullptr;
@@ -121,6 +119,7 @@ JSObject* ProxyObject::getHandlerTrap(JSGlobalObject* globalObject, JSObject* ha
 
     bool isSlotCacheable = slot.isUnset() || (slot.isCacheableValue() && slot.slotBase() == handler);
     if (isSlotCacheable) {
+        JSValue handlerPrototype = handler->getPrototypeDirect();
         bool isHandlerPrototypeChainCacheable = handler->type() == FinalObjectType && !handler->structure()->isDictionary()
             && handlerPrototype.inherits<ObjectPrototype>() && !asObject(handlerPrototype)->structure()->isDictionary();
         if (isHandlerPrototypeChainCacheable) {
@@ -286,9 +285,9 @@ bool ProxyObject::performInternalMethodGetOwnProperty(JSGlobalObject* globalObje
 
     JSObject* handler = jsCast<JSObject*>(handlerValue);
     CallData callData;
-    JSValue getOwnPropertyDescriptorMethod = handler->getMethod(globalObject, callData, makeIdentifier(vm, "getOwnPropertyDescriptor"_s), "'getOwnPropertyDescriptor' property of a Proxy's handler should be callable"_s);
+    JSObject* getOwnPropertyDescriptorMethod = getHandlerTrap(globalObject, handler, callData, vm.propertyNames->getOwnPropertyDescriptor, HandlerTrap::GetOwnPropertyDescriptor);
     RETURN_IF_EXCEPTION(scope, false);
-    if (getOwnPropertyDescriptorMethod.isUndefined())
+    if (!getOwnPropertyDescriptorMethod)
         RELEASE_AND_RETURN(scope, performDefaultGetOwnProperty());
 
     MarkedArgumentBuffer arguments;
@@ -1000,6 +999,26 @@ bool ProxyObject::defineOwnProperty(JSObject* object, JSGlobalObject* globalObje
     return thisObject->performDefineOwnProperty(globalObject, propertyName, descriptor, shouldThrow);
 }
 
+bool ProxyObject::forwardsGetOwnPropertyNamesToTarget(DontEnumPropertiesMode dontEnumPropertiesMode)
+{
+    JSValue handler = this->handler();
+    if (handler.isNull())
+        return false;
+
+    if (!isHandlerTrapsCacheValid(asObject(handler)))
+        return false;
+
+    if (m_handlerTrapsOffsetsCache[static_cast<uint8_t>(HandlerTrap::OwnKeys)] != invalidOffset)
+        return false;
+
+    if (dontEnumPropertiesMode == DontEnumPropertiesMode::Exclude) {
+        if (m_handlerTrapsOffsetsCache[static_cast<uint8_t>(HandlerTrap::GetOwnPropertyDescriptor)] != invalidOffset)
+            return false;
+    }
+
+    return true;
+}
+
 void ProxyObject::performGetOwnPropertyNames(JSGlobalObject* globalObject, PropertyNameArray& propertyNames)
 {
     NO_TAIL_CALLS();
@@ -1018,10 +1037,10 @@ void ProxyObject::performGetOwnPropertyNames(JSGlobalObject* globalObject, Prope
 
     JSObject* handler = jsCast<JSObject*>(handlerValue);
     CallData callData;
-    JSValue ownKeysMethod = handler->getMethod(globalObject, callData, makeIdentifier(vm, "ownKeys"_s), "'ownKeys' property of a Proxy's handler should be callable"_s);
+    JSObject* ownKeysMethod = getHandlerTrap(globalObject, handler, callData, vm.propertyNames->ownKeys, HandlerTrap::OwnKeys);
     RETURN_IF_EXCEPTION(scope, void());
     JSObject* target = this->target();
-    if (ownKeysMethod.isUndefined()) {
+    if (!ownKeysMethod) {
         scope.release();
         target->methodTable()->getOwnPropertyNames(target, globalObject, propertyNames, DontEnumPropertiesMode::Include);
         return;
@@ -1058,6 +1077,9 @@ void ProxyObject::performGetOwnPropertyNames(JSGlobalObject* globalObject, Prope
     });
     RETURN_IF_EXCEPTION(scope, void());
 
+    if (!target->structure()->isNonExtensibleOrHasNonConfigurableProperties())
+        return;
+
     bool targetIsExensible = target->isExtensible(globalObject);
     RETURN_IF_EXCEPTION(scope, void());
 

diff --git a/Source/JavaScriptCore/runtime/ProxyObject.h b/Source/JavaScriptCore/runtime/ProxyObject.h
@@ -51,10 +51,12 @@ class ProxyObject final : public JSInternalFieldObjectImpl<2> {
     enum class HandlerTrap : uint8_t {
         Has = 0,
         Get,
+        GetOwnPropertyDescriptor,
+        OwnKeys,
         Set,
     };
 
-    static constexpr unsigned numberOfCachedHandlerTrapsOffsets = 3;
+    static constexpr unsigned numberOfCachedHandlerTrapsOffsets = 5;
 
     template<typename CellType, SubspaceAccess mode>
     static GCClient::IsoSubspace* subspaceFor(VM& vm)
@@ -102,12 +104,14 @@ class ProxyObject final : public JSInternalFieldObjectImpl<2> {
     WriteBarrier<Unknown>& internalField(Field field) { return Base::internalField(static_cast<uint32_t>(field)); }
 
     JSObject* getHandlerTrap(JSGlobalObject*, JSObject*, CallData&, const Identifier&, HandlerTrap);
+    bool forwardsGetOwnPropertyNamesToTarget(DontEnumPropertiesMode);
 
 private:
     JS_EXPORT_PRIVATE ProxyObject(VM&, Structure*);
     JS_EXPORT_PRIVATE void finishCreation(VM&, JSGlobalObject*, JSValue target, JSValue handler);
     JS_EXPORT_PRIVATE static Structure* structureForTarget(JSGlobalObject*, JSValue target);
 
+    bool isHandlerTrapsCacheValid(JSObject* handler);
     void clearHandlerTrapsOffsetsCache();
 
     static bool getOwnPropertySlot(JSObject*, JSGlobalObject*, PropertyName, PropertySlot&);
@@ -146,4 +150,9 @@ class ProxyObject final : public JSInternalFieldObjectImpl<2> {
     WriteBarrierStructureID m_handlerPrototypeStructureID;
 };
 
+ALWAYS_INLINE bool ProxyObject::isHandlerTrapsCacheValid(JSObject* handler)
+{
+    return handler->structureID() == m_handlerStructureID.value() && asObject(handler->getPrototypeDirect())->structureID() == m_handlerPrototypeStructureID.value();
+}
+
 } // namespace JSC