Diff
Modified: trunk/JSTests/ChangeLog (275362 => 275363)
--- trunk/JSTests/ChangeLog 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/JSTests/ChangeLog 2021-04-01 16:07:08 UTC (rev 275363)
@@ -1,3 +1,16 @@
+2021-04-01 Alexey Shvayka <[email protected]>
+
+ Optimize createListFromArrayLike() and Proxy's [[OwnPropertyKeys]] method
+ https://bugs.webkit.org/show_bug.cgi?id=223928
+
+ Reviewed by Yusuke Suzuki.
+
+ * microbenchmarks/json-stringify-array-replacer.js:
+ Reduce running time from over 350ms to ~60ms.
+
+ * microbenchmarks/reflect-own-keys-proxy-2.js: Added.
+ * microbenchmarks/reflect-own-keys-proxy.js: Added.
+
2021-03-31 Mark Lam <[email protected]>
Missing exception check in HashMapImpl::add().
Modified: trunk/JSTests/microbenchmarks/json-stringify-array-replacer.js (275362 => 275363)
--- trunk/JSTests/microbenchmarks/json-stringify-array-replacer.js 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/JSTests/microbenchmarks/json-stringify-array-replacer.js 2021-04-01 16:07:08 UTC (rev 275363)
@@ -1,3 +1,3 @@
-const replacer = new Array(15).fill('key');
-for (let i = 0; i < 1e6; ++i)
+var replacer = new Array(25).fill('key');
+for (var i = 0; i < 1e5; ++i)
JSON.stringify(null, replacer);
Added: trunk/JSTests/microbenchmarks/reflect-own-keys-proxy-2.js (0 => 275363)
--- trunk/JSTests/microbenchmarks/reflect-own-keys-proxy-2.js (rev 0)
+++ trunk/JSTests/microbenchmarks/reflect-own-keys-proxy-2.js 2021-04-01 16:07:08 UTC (rev 275363)
@@ -0,0 +1,16 @@
+(function() {
+ var target = {};
+ for (var i = 0; i < 10; i++)
+ target["k" + i] = i;
+ var trapResult = Reflect.ownKeys(target);
+ var proxy = new Proxy(target, {
+ ownKeys: function() { return trapResult; },
+ });
+
+ var j = 0, lengthSum = 0;
+ for (; j < 50_000; ++j)
+ lengthSum += Reflect.ownKeys(proxy).length;
+
+ if (lengthSum !== trapResult.length * j)
+ throw "Bad assertion!";
+})();
Added: trunk/JSTests/microbenchmarks/reflect-own-keys-proxy.js (0 => 275363)
--- trunk/JSTests/microbenchmarks/reflect-own-keys-proxy.js (rev 0)
+++ trunk/JSTests/microbenchmarks/reflect-own-keys-proxy.js 2021-04-01 16:07:08 UTC (rev 275363)
@@ -0,0 +1,13 @@
+(function() {
+ var trapResult = [];
+ var proxy = new Proxy({}, {
+ ownKeys: function() { return trapResult; },
+ });
+
+ var j = 0, lengthSum = 0;
+ for (; j < 200_000; ++j)
+ lengthSum += Reflect.ownKeys(proxy).length;
+
+ if (lengthSum !== trapResult.length * j)
+ throw "Bad assertion!";
+})();
Modified: trunk/Source/_javascript_Core/CMakeLists.txt (275362 => 275363)
--- trunk/Source/_javascript_Core/CMakeLists.txt 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/CMakeLists.txt 2021-04-01 16:07:08 UTC (rev 275363)
@@ -933,6 +933,7 @@
runtime/IteratorOperations.h
runtime/IteratorPrototype.h
runtime/JSArray.h
+ runtime/JSArrayInlines.h
runtime/JSArrayBuffer.h
runtime/JSArrayBufferPrototype.h
runtime/JSArrayBufferView.h
Modified: trunk/Source/_javascript_Core/ChangeLog (275362 => 275363)
--- trunk/Source/_javascript_Core/ChangeLog 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-04-01 16:07:08 UTC (rev 275363)
@@ -1,3 +1,44 @@
+2021-04-01 Alexey Shvayka <[email protected]>
+
+ Optimize createListFromArrayLike() and Proxy's [[OwnPropertyKeys]] method
+ https://bugs.webkit.org/show_bug.cgi?id=223928
+
+ Reviewed by Yusuke Suzuki.
+
+ createListFromArrayLike() changes:
+
+ 1. Use toLength() / getIndex() methods that have fast paths.
+ 2. Remove RuntimeTypeMask and error messages from its signature: type checks are better
+ performed in advance / inside a functor to keep the helper more versatile.
+ 3. Invert functor's return value to align with Structure::forEachProperty() and friends.
+ 4. Rename it to forEachInArrayLike() as no list is actually returned.
+
+ ProxyObject::performGetOwnPropertyNames() changes:
+
+ 1. Remove RuntimeTypeMask filtering as it's already performed by PropertyNameArray::add().
+ 2. Store target's keys in a HashSet for faster insertion / search.
+ 3. Don't populate `targetConfigurableKeys` for extensible target as it won't be used [1].
+ 4. Leverage return value of HashSet::remove() instead of using a helper.
+
+ This patch advances Proxy's [[OwnPropertyKeys]] microbenchmarks by 20-30%,
+ mainly due to createListFromArrayLike() changes. No behavior changes.
+
+ Also, utilizes forEachInArrayLike() for allow list of JSON.stringify().
+
+ [1]: https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys (step 20)
+
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::Stringifier):
+ * runtime/JSObject.h:
+ (JSC::JSObject::getIndex const):
+ * runtime/JSObjectInlines.h:
+ (JSC::forEachInArrayLike):
+ (JSC::createListFromArrayLike): Deleted.
+ * runtime/ProxyObject.cpp:
+ (JSC::ProxyObject::performGetOwnPropertyNames):
+ * runtime/ReflectObject.cpp:
+ (JSC::JSC_DEFINE_HOST_FUNCTION):
+
2021-03-31 David Kilzer <[email protected]>
UBSan: JSC::Parser<LexerType>::parseProperty(): runtime error: load of value nnn, which is not a valid value for type 'bool'
Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (275362 => 275363)
--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2021-04-01 16:07:08 UTC (rev 275363)
@@ -1098,7 +1098,7 @@
539BFBAE22AD3C3A0023F4C0 /* WeakObjectRefPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = 539BFBAD22AD3C3A0023F4C0 /* WeakObjectRefPrototype.h */; };
539BFBB022AD3CDC0023F4C0 /* JSWeakObjectRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 539BFBAF22AD3CDC0023F4C0 /* JSWeakObjectRef.h */; };
539DD7F523C1BBB500905F13 /* JSArrayIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = 539DD7F423C1BBA900905F13 /* JSArrayIterator.h */; settings = {ATTRIBUTES = (Private, ); }; };
- 539FB8BA1C99DA7C00940FA1 /* JSArrayInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 539FB8B91C99DA7C00940FA1 /* JSArrayInlines.h */; };
+ 539FB8BA1C99DA7C00940FA1 /* JSArrayInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 539FB8B91C99DA7C00940FA1 /* JSArrayInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
53B4BD121F68B32500D2BEA3 /* WasmOps.h in Headers */ = {isa = PBXBuildFile; fileRef = 533B15DE1DC7F463004D500A /* WasmOps.h */; settings = {ATTRIBUTES = (Private, ); }; };
53B601EC2034B8C5006BE667 /* JSCast.h in Headers */ = {isa = PBXBuildFile; fileRef = 53B601EB2034B8C5006BE667 /* JSCast.h */; settings = {ATTRIBUTES = (Private, ); }; };
53C2CE9C22FCC3D6008B2853 /* AirHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = 53C2CE9B22FCC3D6008B2853 /* AirHelpers.h */; };
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (275362 => 275363)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2021-04-01 16:07:08 UTC (rev 275363)
@@ -231,28 +231,22 @@
RETURN_IF_EXCEPTION(scope, );
if (isArrayReplacer) {
m_usingArrayReplacer = true;
- uint64_t length = static_cast<uint64_t>(toLength(globalObject, replacerObject));
- RETURN_IF_EXCEPTION(scope, );
- for (uint64_t index = 0; index < length; ++index) {
- JSValue name;
- if (isJSArray(replacerObject) && replacerObject->canGetIndexQuickly(index))
- name = replacerObject->getIndexQuickly(static_cast<uint32_t>(index));
- else {
- name = replacerObject->get(globalObject, index);
- RETURN_IF_EXCEPTION(scope, );
- }
+ forEachInArrayLike(globalObject, replacerObject, [&] (JSValue name) -> bool {
if (name.isObject()) {
auto* nameObject = jsCast<JSObject*>(name);
if (!nameObject->inherits<NumberObject>(vm) && !nameObject->inherits<StringObject>(vm))
- continue;
+ return true;
} else if (!name.isNumber() && !name.isString())
- continue;
+ return true;
+
JSString* propertyNameString = name.toString(globalObject);
- RETURN_IF_EXCEPTION(scope, );
+ RETURN_IF_EXCEPTION(scope, false);
auto propertyName = propertyNameString->toIdentifier(globalObject);
- RETURN_IF_EXCEPTION(scope, );
+ RETURN_IF_EXCEPTION(scope, false);
m_arrayReplacerPropertyNames.add(WTFMove(propertyName));
- }
+ return true;
+ });
+ RETURN_IF_EXCEPTION(scope, );
}
}
}
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (275362 => 275363)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2021-04-01 16:07:08 UTC (rev 275363)
@@ -404,7 +404,7 @@
return JSValue();
}
- JSValue getIndex(JSGlobalObject* globalObject, unsigned i) const
+ JSValue getIndex(JSGlobalObject* globalObject, uint64_t i) const
{
if (JSValue result = tryGetIndexQuickly(i))
return result;
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (275362 => 275363)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2021-04-01 16:07:08 UTC (rev 275363)
@@ -27,6 +27,7 @@
#include "BrandedStructure.h"
#include "ButterflyInlines.h"
#include "Error.h"
+#include "JSArrayInlines.h"
#include "JSFunction.h"
#include "JSObject.h"
#include "JSTypedArrays.h"
@@ -43,38 +44,19 @@
return &vm.cellSpace;
}
-// Section 7.3.17 of the spec.
-template <typename AddFunction> // Add function should have a type like: (JSValue, RuntimeType) -> bool
-void createListFromArrayLike(JSGlobalObject* globalObject, JSValue arrayLikeValue, RuntimeTypeMask legalTypesFilter, const String& notAnObjectErroMessage, const String& illegalTypeErrorMessage, AddFunction addFunction)
+// https://tc39.es/ecma262/#sec-createlistfromarraylike
+template <typename Functor> // A functor should have a type like: (JSValue) -> bool
+void forEachInArrayLike(JSGlobalObject* globalObject, JSObject* arrayLikeObject, Functor functor)
{
VM& vm = getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
-
- if (!arrayLikeValue.isObject()) {
- throwTypeError(globalObject, scope, notAnObjectErroMessage);
- return;
- }
-
- Vector<JSValue> result;
- JSValue lengthProperty = arrayLikeValue.get(globalObject, vm.propertyNames->length);
+ uint64_t length = static_cast<uint64_t>(toLength(globalObject, arrayLikeObject));
RETURN_IF_EXCEPTION(scope, void());
- double lengthAsDouble = lengthProperty.toLength(globalObject);
- RETURN_IF_EXCEPTION(scope, void());
- RELEASE_ASSERT(lengthAsDouble >= 0.0 && lengthAsDouble == std::trunc(lengthAsDouble));
- uint64_t length = static_cast<uint64_t>(lengthAsDouble);
for (uint64_t index = 0; index < length; index++) {
- JSValue next = arrayLikeValue.get(globalObject, index);
+ JSValue value = arrayLikeObject->getIndex(globalObject, index);
RETURN_IF_EXCEPTION(scope, void());
-
- RuntimeType type = runtimeTypeForValue(vm, next);
- if (!(type & legalTypesFilter)) {
- throwTypeError(globalObject, scope, illegalTypeErrorMessage);
+ if (!functor(value))
return;
- }
-
- bool exitEarly = addFunction(next, type);
- if (exitEarly)
- return;
}
}
Modified: trunk/Source/_javascript_Core/runtime/ProxyObject.cpp (275362 => 275363)
--- trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2021-04-01 16:07:08 UTC (rev 275363)
@@ -923,45 +923,31 @@
JSValue trapResult = call(globalObject, ownKeysMethod, callData, handler, arguments);
RETURN_IF_EXCEPTION(scope, void());
+ if (!trapResult.isObject()) {
+ throwTypeError(globalObject, scope, "Proxy handler's 'ownKeys' method must return an object"_s);
+ return;
+ }
+
HashSet<UniquedStringImpl*> uncheckedResultKeys;
- {
- RuntimeTypeMask resultFilter = 0;
- switch (propertyNames.propertyNameMode()) {
- case PropertyNameMode::Symbols:
- resultFilter = TypeSymbol;
- break;
- case PropertyNameMode::Strings:
- resultFilter = TypeString;
- break;
- case PropertyNameMode::StringsAndSymbols:
- resultFilter = TypeSymbol | TypeString;
- break;
+ forEachInArrayLike(globalObject, asObject(trapResult), [&] (JSValue value) -> bool {
+ if (!value.isString() && !value.isSymbol()) {
+ throwTypeError(globalObject, scope, "Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"_s);
+ return false;
}
- ASSERT(resultFilter);
- auto addPropName = [&] (JSValue value, RuntimeType type) -> bool {
- static constexpr bool doExitEarly = true;
- static constexpr bool dontExitEarly = false;
+ Identifier ident = value.toPropertyKey(globalObject);
+ RETURN_IF_EXCEPTION(scope, false);
- Identifier ident = value.toPropertyKey(globalObject);
- RETURN_IF_EXCEPTION(scope, doExitEarly);
+ if (!uncheckedResultKeys.add(ident.impl()).isNewEntry) {
+ throwTypeError(globalObject, scope, "Proxy handler's 'ownKeys' trap result must not contain any duplicate names"_s);
+ return false;
+ }
- if (!uncheckedResultKeys.add(ident.impl()).isNewEntry) {
- throwTypeError(globalObject, scope, "Proxy handler's 'ownKeys' trap result must not contain any duplicate names"_s);
- return doExitEarly;
- }
+ propertyNames.add(ident);
+ return true;
+ });
+ RETURN_IF_EXCEPTION(scope, void());
- if (type & resultFilter)
- propertyNames.add(ident.impl());
-
- return dontExitEarly;
- };
-
- RuntimeTypeMask dontThrowAnExceptionTypeFilter = TypeString | TypeSymbol;
- createListFromArrayLike(globalObject, trapResult, dontThrowAnExceptionTypeFilter, "Proxy handler's 'ownKeys' method must return an object"_s, "Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"_s, addPropName);
- RETURN_IF_EXCEPTION(scope, void());
- }
-
bool targetIsExensible = target->isExtensible(globalObject);
RETURN_IF_EXCEPTION(scope, void());
@@ -968,30 +954,20 @@
PropertyNameArray targetKeys(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
target->methodTable(vm)->getOwnPropertyNames(target, globalObject, targetKeys, DontEnumPropertiesMode::Include);
RETURN_IF_EXCEPTION(scope, void());
- Vector<UniquedStringImpl*> targetConfigurableKeys;
- Vector<UniquedStringImpl*> targetNonConfigurableKeys;
+ HashSet<UniquedStringImpl*> targetNonConfigurableKeys;
+ HashSet<UniquedStringImpl*> targetConfigurableKeys;
for (const Identifier& ident : targetKeys) {
PropertyDescriptor descriptor;
bool isPropertyDefined = target->getOwnPropertyDescriptor(globalObject, ident.impl(), descriptor);
RETURN_IF_EXCEPTION(scope, void());
if (isPropertyDefined && !descriptor.configurable())
- targetNonConfigurableKeys.append(ident.impl());
- else
- targetConfigurableKeys.append(ident.impl());
+ targetNonConfigurableKeys.add(ident.impl());
+ else if (!targetIsExensible)
+ targetConfigurableKeys.add(ident.impl());
}
- enum ContainedIn { IsContainedIn, IsNotContainedIn };
- auto removeIfContainedInUncheckedResultKeys = [&] (UniquedStringImpl* impl) -> ContainedIn {
- auto iter = uncheckedResultKeys.find(impl);
- if (iter == uncheckedResultKeys.end())
- return IsNotContainedIn;
-
- uncheckedResultKeys.remove(iter);
- return IsContainedIn;
- };
-
for (UniquedStringImpl* impl : targetNonConfigurableKeys) {
- if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
+ if (!uncheckedResultKeys.remove(impl)) {
throwTypeError(globalObject, scope, makeString("Proxy object's 'target' has the non-configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
return;
}
@@ -999,7 +975,7 @@
if (!targetIsExensible) {
for (UniquedStringImpl* impl : targetConfigurableKeys) {
- if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
+ if (!uncheckedResultKeys.remove(impl)) {
throwTypeError(globalObject, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
return;
}
Modified: trunk/Source/_javascript_Core/runtime/ReflectObject.cpp (275362 => 275363)
--- trunk/Source/_javascript_Core/runtime/ReflectObject.cpp 2021-04-01 15:48:11 UTC (rev 275362)
+++ trunk/Source/_javascript_Core/runtime/ReflectObject.cpp 2021-04-01 16:07:08 UTC (rev 275363)
@@ -110,9 +110,9 @@
if (!argumentsObject)
return JSValue::encode(throwTypeError(globalObject, scope, "Reflect.construct requires the second argument be an object"_s));
- createListFromArrayLike(globalObject, argumentsObject, RuntimeTypeMaskAllTypes, "This error must not be raised"_s, "This error must not be raised"_s, [&] (JSValue value, RuntimeType) -> bool {
+ forEachInArrayLike(globalObject, argumentsObject, [&] (JSValue value) -> bool {
arguments.append(value);
- return false;
+ return true;
});
RETURN_IF_EXCEPTION(scope, (arguments.overflowCheckNotNeeded(), encodedJSValue()));
if (UNLIKELY(arguments.hasOverflowed())) {
