Diff
Modified: trunk/JSTests/ChangeLog (272637 => 272638)
--- trunk/JSTests/ChangeLog 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/JSTests/ChangeLog 2021-02-10 05:46:03 UTC (rev 272638)
@@ -1,3 +1,18 @@
+2021-02-09 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] C++ iteration should support fast iterator protocol
+ https://bugs.webkit.org/show_bug.cgi?id=221526
+
+ Reviewed by Alexey Shvayka.
+
+ * microbenchmarks/map-constructor.js: Added.
+ (test):
+ * stress/map-constructor-hole-throw.js: Added.
+ (shouldBe):
+ (shouldThrow):
+ (values.__proto__.return):
+ (Map.prototype):
+
2021-02-09 Caio Lima <ticaiol...@gmail.com>
Fix warning introduced by r272580
Added: trunk/JSTests/microbenchmarks/map-constructor.js (0 => 272638)
--- trunk/JSTests/microbenchmarks/map-constructor.js (rev 0)
+++ trunk/JSTests/microbenchmarks/map-constructor.js 2021-02-10 05:46:03 UTC (rev 272638)
@@ -0,0 +1,31 @@
+function test(array)
+{
+ return new Map(array);
+}
+noInline(test);
+
+var array = [
+ [0, 0],
+ [1, 0],
+ [2, 0],
+ [3, 0],
+ [4, 0],
+ [5, 0],
+ [6, 0],
+ [7, 0],
+ [8, 0],
+ [9, 0],
+ [10, 0],
+ [11, 0],
+ [12, 0],
+ [13, 0],
+ [14, 0],
+ [15, 0],
+ [16, 0],
+ [17, 0],
+ [18, 0],
+ [19, 0],
+];
+
+for (var i = 0; i < 1e4; ++i)
+ test(array);
Added: trunk/JSTests/stress/map-constructor-hole-throw-and-iterator-resume.js (0 => 272638)
--- trunk/JSTests/stress/map-constructor-hole-throw-and-iterator-resume.js (rev 0)
+++ trunk/JSTests/stress/map-constructor-hole-throw-and-iterator-resume.js 2021-02-10 05:46:03 UTC (rev 272638)
@@ -0,0 +1,52 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, errorMessage) {
+ var errorThrown = false;
+ var error = null;
+ try {
+ func();
+ } catch (e) {
+ errorThrown = true;
+ error = e;
+ }
+ if (!errorThrown)
+ throw new Error('not thrown');
+ if (String(error) !== errorMessage)
+ throw new Error(`bad error: ${String(error)}`);
+}
+
+var iterator;
+[].values().__proto__.return = function () {
+ iterator = this;
+};
+
+var array = [
+ [0, 0],
+ [1, 0],
+ [2, 0],
+];
+var counter = 0;
+Map.prototype.set = function () {
+ if (++counter >= 3)
+ throw new Error("ok");
+};
+shouldThrow(() => {
+ var map = new Map(array);
+}, `Error: ok`);
+shouldBe(iterator !== undefined, true);
+shouldBe(iterator.next().done, true);
+
+counter = 0;
+iterator = undefined;
+shouldThrow(() => {
+ var map = new Map(array);
+}, `Error: ok`);
+shouldBe(iterator !== undefined, true);
+array.push([3, 0], [4, 0], [5, 0]);
+var { done, value } = iterator.next();
+shouldBe(done, false);
+shouldBe(value, array[3]);
+
Added: trunk/JSTests/stress/map-constructor-hole-throw.js (0 => 272638)
--- trunk/JSTests/stress/map-constructor-hole-throw.js (rev 0)
+++ trunk/JSTests/stress/map-constructor-hole-throw.js 2021-02-10 05:46:03 UTC (rev 272638)
@@ -0,0 +1,39 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, errorMessage) {
+ var errorThrown = false;
+ var error = null;
+ try {
+ func();
+ } catch (e) {
+ errorThrown = true;
+ error = e;
+ }
+ if (!errorThrown)
+ throw new Error('not thrown');
+ if (String(error) !== errorMessage)
+ throw new Error(`bad error: ${String(error)}`);
+}
+
+var iterator;
+[].values().__proto__.return = function () {
+ iterator = this;
+};
+
+var array = [
+ [0, 0],
+ [1, 0],
+ [2, 0],
+];
+Map.prototype.set = function () {
+ throw new Error("ok");
+};
+shouldThrow(() => {
+ var map = new Map(array);
+}, `Error: ok`);
+shouldBe(iterator !== undefined, true);
+shouldBe(iterator.next().value, array[1]);
+
Modified: trunk/Source/_javascript_Core/CMakeLists.txt (272637 => 272638)
--- trunk/Source/_javascript_Core/CMakeLists.txt 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/CMakeLists.txt 2021-02-10 05:46:03 UTC (rev 272638)
@@ -562,6 +562,7 @@
bytecode/Instruction.h
bytecode/InstructionStream.h
bytecode/InternalFunctionAllocationProfile.h
+ bytecode/IterationModeMetadata.h
bytecode/JumpTable.h
bytecode/LLIntCallLinkInfo.h
bytecode/LLIntPrototypeLoadAdaptiveStructureWatchpoint.h
@@ -927,6 +928,7 @@
runtime/JSArrayBufferPrototype.h
runtime/JSArrayBufferView.h
runtime/JSArrayBufferViewInlines.h
+ runtime/JSArrayIterator.h
runtime/JSBigInt.h
runtime/JSCConfig.h
runtime/JSCInlines.h
Modified: trunk/Source/_javascript_Core/ChangeLog (272637 => 272638)
--- trunk/Source/_javascript_Core/ChangeLog 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-02-10 05:46:03 UTC (rev 272638)
@@ -1,3 +1,35 @@
+2021-02-09 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] C++ iteration should support fast iterator protocol
+ https://bugs.webkit.org/show_bug.cgi?id=221526
+
+ Reviewed by Alexey Shvayka.
+
+ This patch adds fast iteration protocol support in C++ iteration (forEachIterable).
+ In JS, we have op_iterator_open / op_iterator_next to iterate array quickly.
+ But we do not use this feature in C++ forEachIterable. In this patch we integrate
+ the same (or a bit more efficient since we can avoid creating JSArrayIterator) mechanism
+ so that iteration in C++ gets faster for normal arrays.
+
+ We observed 1.9x faster in Map creation with arrays.
+
+ ToT Patched
+
+ map-constructor 35.7446+-0.2354 ^ 18.7516+-0.4534 ^ definitely 1.9062x faster
+
+ * CMakeLists.txt:
+ * _javascript_Core.xcodeproj/project.pbxproj:
+ * inspector/JSInjectedScriptHost.cpp:
+ (Inspector::JSInjectedScriptHost::iteratorEntries):
+ * runtime/CommonSlowPaths.cpp:
+ (JSC::iteratorOpenTryFastImpl):
+ * runtime/IteratorOperations.cpp:
+ (JSC::iteratorClose):
+ (JSC::prepareForFastArrayIteration):
+ * runtime/IteratorOperations.h:
+ (JSC::forEachInIterable):
+ * runtime/JSArrayIterator.h:
+
2021-02-09 Caio Lima <ticaiol...@gmail.com>
Fix warning introduced by r272580
Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (272637 => 272638)
--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2021-02-10 05:46:03 UTC (rev 272638)
@@ -1097,7 +1097,7 @@
539930C822AD3B9A0051CDE2 /* WeakObjectRefConstructor.h in Headers */ = {isa = PBXBuildFile; fileRef = 539930C722AD3B9A0051CDE2 /* WeakObjectRefConstructor.h */; };
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 */; };
+ 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 */; };
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, ); }; };
Modified: trunk/Source/_javascript_Core/inspector/JSInjectedScriptHost.cpp (272637 => 272638)
--- trunk/Source/_javascript_Core/inspector/JSInjectedScriptHost.cpp 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/inspector/JSInjectedScriptHost.cpp 2021-02-10 05:46:03 UTC (rev 272638)
@@ -613,7 +613,7 @@
array->putDirectIndex(globalObject, i, entry);
if (UNLIKELY(scope.exception())) {
scope.release();
- iteratorClose(globalObject, iterationRecord);
+ iteratorClose(globalObject, iterationRecord.iterator);
break;
}
}
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (272637 => 272638)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2021-02-10 05:46:03 UTC (rev 272638)
@@ -34,6 +34,7 @@
#include "ErrorHandlingScope.h"
#include "ExceptionFuzz.h"
#include "FrameTracers.h"
+#include "IteratorOperations.h"
#include "JSArrayIterator.h"
#include "JSAsyncGenerator.h"
#include "JSCInlines.h"
@@ -863,20 +864,7 @@
JSValue symbolIterator = GET_C(bytecode.m_symbolIterator).jsValue();
auto& iterator = GET(bytecode.m_iterator);
- auto prepareForFastArrayIteration = [&] {
- if (!globalObject->arrayIteratorProtocolWatchpointSet().isStillValid())
- return IterationMode::Generic;
-
- // This is correct because we just checked the watchpoint is still valid.
- JSFunction* symbolIteratorFunction = jsDynamicCast<JSFunction*>(vm, symbolIterator);
- if (!symbolIteratorFunction)
- return IterationMode::Generic;
-
- // We don't want to allocate the values function just to check if it's the same as our function at so we use the concurrent accessor.
- // FIXME: This only works for arrays from the same global object as ourselves but we should be able to support any pairing.
- if (globalObject->arrayProtoValuesFunctionConcurrently() != symbolIteratorFunction)
- return IterationMode::Generic;
-
+ if (getIterationMode(vm, globalObject, iterable, symbolIterator) == IterationMode::FastArray) {
// We should be good to go.
metadata.m_iterationMetadata.seenModes = metadata.m_iterationMetadata.seenModes | IterationMode::FastArray;
GET(bytecode.m_next) = JSValue();
@@ -883,17 +871,12 @@
auto* iteratedObject = jsCast<JSObject*>(iterable);
iterator = JSArrayIterator::create(vm, globalObject->arrayIteratorStructure(), iteratedObject, IterationKind::Values);
PROFILE_VALUE_IN(iterator.jsValue(), m_iteratorProfile);
- return IterationMode::FastArray;
- };
-
- if (iterable.inherits<JSArray>(vm)) {
- if (prepareForFastArrayIteration() == IterationMode::FastArray)
- return encodeResult(pc, reinterpret_cast<void*>(IterationMode::FastArray));
+ return encodeResult(pc, reinterpret_cast<void*>(static_cast<uintptr_t>(IterationMode::FastArray)));
}
// Return to the bytecode to try in generic mode.
metadata.m_iterationMetadata.seenModes = metadata.m_iterationMetadata.seenModes | IterationMode::Generic;
- return encodeResult(pc, reinterpret_cast<void*>(IterationMode::Generic));
+ return encodeResult(pc, reinterpret_cast<void*>(static_cast<uintptr_t>(IterationMode::Generic)));
}
JSC_DEFINE_COMMON_SLOW_PATH(iterator_open_try_fast_narrow)
Modified: trunk/Source/_javascript_Core/runtime/IteratorOperations.cpp (272637 => 272638)
--- trunk/Source/_javascript_Core/runtime/IteratorOperations.cpp 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/runtime/IteratorOperations.cpp 2021-02-10 05:46:03 UTC (rev 272638)
@@ -82,7 +82,7 @@
return result;
}
-void iteratorClose(JSGlobalObject* globalObject, IterationRecord iterationRecord)
+void iteratorClose(JSGlobalObject* globalObject, JSValue iterator)
{
VM& vm = globalObject->vm();
auto throwScope = DECLARE_THROW_SCOPE(vm);
@@ -94,7 +94,7 @@
catchScope.clearException();
}
- JSValue returnFunction = iterationRecord.iterator.get(globalObject, vm.propertyNames->returnKeyword);
+ JSValue returnFunction = iterator.get(globalObject, vm.propertyNames->returnKeyword);
if (UNLIKELY(throwScope.exception()) || returnFunction.isUndefinedOrNull()) {
if (exception)
throwException(globalObject, throwScope, exception);
@@ -112,7 +112,7 @@
MarkedArgumentBuffer returnFunctionArguments;
ASSERT(!returnFunctionArguments.hasOverflowed());
- JSValue innerResult = call(globalObject, returnFunction, returnFunctionCallData, iterationRecord.iterator, returnFunctionArguments);
+ JSValue innerResult = call(globalObject, returnFunction, returnFunctionCallData, iterator, returnFunctionArguments);
if (exception) {
throwException(globalObject, throwScope, exception);
@@ -233,4 +233,48 @@
return { iterator, nextMethod };
}
+IterationMode getIterationMode(VM& vm, JSGlobalObject* globalObject, JSValue iterable, JSValue symbolIterator)
+{
+ if (!iterable.inherits<JSArray>(vm))
+ return IterationMode::Generic;
+
+ if (!globalObject->arrayIteratorProtocolWatchpointSet().isStillValid())
+ return IterationMode::Generic;
+
+ // This is correct because we just checked the watchpoint is still valid.
+ JSFunction* symbolIteratorFunction = jsDynamicCast<JSFunction*>(vm, symbolIterator);
+ if (!symbolIteratorFunction)
+ return IterationMode::Generic;
+
+ // We don't want to allocate the values function just to check if it's the same as our function so we use the concurrent accessor.
+ // FIXME: This only works for arrays from the same global object as ourselves but we should be able to support any pairing.
+ if (globalObject->arrayProtoValuesFunctionConcurrently() != symbolIteratorFunction)
+ return IterationMode::Generic;
+
+ return IterationMode::FastArray;
+}
+
+IterationMode getIterationMode(VM& vm, JSGlobalObject* globalObject, JSValue iterable)
+{
+ if (!iterable.inherits<JSArray>(vm))
+ return IterationMode::Generic;
+
+ JSArray* array = jsCast<JSArray*>(iterable);
+ Structure* structure = array->structure(vm);
+ // FIXME: We want to support broader JSArrays as long as array[@@iterator] is not defined.
+ if (!globalObject->isOriginalArrayStructure(structure))
+ return IterationMode::Generic;
+
+ if (!globalObject->arrayIteratorProtocolWatchpointSet().isStillValid())
+ return IterationMode::Generic;
+
+ // Now, Array has original Array Structures and arrayIteratorProtocolWatchpointSet is not fired.
+ // This means,
+ // 1. Array.prototype is [[Prototype]].
+ // 2. array[@@iterator] is not overridden.
+ // 3. Array.prototype[@@iterator] is an expected one.
+ // So, we can say this will create an expected ArrayIterator.
+ return IterationMode::FastArray;
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/IteratorOperations.h (272637 => 272638)
--- trunk/Source/_javascript_Core/runtime/IteratorOperations.h 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/runtime/IteratorOperations.h 2021-02-10 05:46:03 UTC (rev 272638)
@@ -26,8 +26,10 @@
#pragma once
+#include "IterationModeMetadata.h"
+#include "JSArrayIterator.h"
#include "JSCJSValue.h"
-#include "JSObject.h"
+#include "JSObjectInlines.h"
#include "ThrowScope.h"
namespace JSC {
@@ -41,7 +43,7 @@
JS_EXPORT_PRIVATE JSValue iteratorValue(JSGlobalObject*, JSValue iterResult);
bool iteratorComplete(JSGlobalObject*, JSValue iterResult);
JS_EXPORT_PRIVATE JSValue iteratorStep(JSGlobalObject*, IterationRecord);
-JS_EXPORT_PRIVATE void iteratorClose(JSGlobalObject*, IterationRecord);
+JS_EXPORT_PRIVATE void iteratorClose(JSGlobalObject*, JSValue iterator);
JS_EXPORT_PRIVATE JSObject* createIteratorResultObject(JSGlobalObject*, JSValue, bool done);
Structure* createIteratorResultObjectStructure(VM&, JSGlobalObject&);
@@ -53,6 +55,9 @@
JS_EXPORT_PRIVATE JSValue iteratorMethod(JSGlobalObject*, JSObject*);
JS_EXPORT_PRIVATE bool hasIteratorMethod(JSGlobalObject*, JSValue);
+JS_EXPORT_PRIVATE IterationMode getIterationMode(VM&, JSGlobalObject*, JSValue iterable);
+JS_EXPORT_PRIVATE IterationMode getIterationMode(VM&, JSGlobalObject*, JSValue iterable, JSValue symbolIterator);
+
template<typename CallBackType>
void forEachInIterable(JSGlobalObject* globalObject, JSValue iterable, const CallBackType& callback)
{
@@ -59,6 +64,23 @@
auto& vm = getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
+ if (getIterationMode(vm, globalObject, iterable) == IterationMode::FastArray) {
+ auto* array = jsCast<JSArray*>(iterable);
+ for (unsigned index = 0; index < array->length(); ++index) {
+ JSValue nextValue = array->getIndex(globalObject, index);
+ RETURN_IF_EXCEPTION(scope, void());
+ callback(vm, globalObject, nextValue);
+ if (UNLIKELY(scope.exception())) {
+ scope.release();
+ JSArrayIterator* iterator = JSArrayIterator::create(vm, globalObject->arrayIteratorStructure(), array, IterationKind::Values);
+ iterator->internalField(JSArrayIterator::Field::Index).setWithoutWriteBarrier(jsNumber(index + 1));
+ iteratorClose(globalObject, iterator);
+ return;
+ }
+ }
+ return;
+ }
+
IterationRecord iterationRecord = iteratorForIterable(globalObject, iterable);
RETURN_IF_EXCEPTION(scope, void());
while (true) {
@@ -72,7 +94,7 @@
callback(vm, globalObject, nextValue);
if (UNLIKELY(scope.exception())) {
scope.release();
- iteratorClose(globalObject, iterationRecord);
+ iteratorClose(globalObject, iterationRecord.iterator);
return;
}
}
@@ -84,6 +106,23 @@
auto& vm = getVM(&globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
+ if (getIterationMode(vm, &globalObject, iterable, iteratorMethod) == IterationMode::FastArray) {
+ auto* array = jsCast<JSArray*>(iterable);
+ for (unsigned index = 0; index < array->length(); ++index) {
+ JSValue nextValue = array->getIndex(&globalObject, index);
+ RETURN_IF_EXCEPTION(scope, void());
+ callback(vm, globalObject, nextValue);
+ if (UNLIKELY(scope.exception())) {
+ scope.release();
+ JSArrayIterator* iterator = JSArrayIterator::create(vm, globalObject.arrayIteratorStructure(), array, IterationKind::Values);
+ iterator->internalField(JSArrayIterator::Field::Index).setWithoutWriteBarrier(jsNumber(index + 1));
+ iteratorClose(&globalObject, iterator);
+ return;
+ }
+ }
+ return;
+ }
+
auto iterationRecord = iteratorForIterable(&globalObject, iterable, iteratorMethod);
RETURN_IF_EXCEPTION(scope, void());
while (true) {
@@ -97,7 +136,7 @@
callback(vm, globalObject, nextValue);
if (UNLIKELY(scope.exception())) {
scope.release();
- iteratorClose(&globalObject, iterationRecord);
+ iteratorClose(&globalObject, iterationRecord.iterator);
return;
}
}
Modified: trunk/Source/_javascript_Core/runtime/JSArrayIterator.h (272637 => 272638)
--- trunk/Source/_javascript_Core/runtime/JSArrayIterator.h 2021-02-10 05:22:35 UTC (rev 272637)
+++ trunk/Source/_javascript_Core/runtime/JSArrayIterator.h 2021-02-10 05:46:03 UTC (rev 272638)
@@ -68,7 +68,7 @@
IterationKind kind() const { return static_cast<IterationKind>(internalField(Field::Kind).get().asUInt32AsAnyInt()); }
JSObject* iteratedObject() const { return jsCast<JSObject*>(internalField(Field::IteratedObject).get()); }
- static JSArrayIterator* create(VM&, Structure*, JSObject* iteratedObject, JSValue kind);
+ JS_EXPORT_PRIVATE static JSArrayIterator* create(VM&, Structure*, JSObject* iteratedObject, JSValue kind);
static JSArrayIterator* create(VM& vm, Structure* structure, JSObject* iteratedObject, IterationKind kind)
{
return create(vm, structure, iteratedObject, jsNumber(static_cast<unsigned>(kind)));
