Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JSC] TypedArray iteration does not need to get "length"
https://bugs.webkit.org/show_bug.cgi?id=243581

Reviewed by Alexey Shvayka.

ArrayIterator#next spec says that if the array is TypedArray, we do not need to look up "length",
and we can directly get TypedArray's internal length. This means that "length" property of instance,
Uint8Array.prototype, and TypedArray.prototype are unrelated to iterator protocol. This makes iterator
protocol guarantee simplified.

This patch applies this change: we no longer ensure "length" validity. We also adjust ArrayIterator#next's
slow path implementation to the spec.

[1]: https://tc39.es/ecma262/#sec-createarrayiterator

* JSTests/stress/typed-array-from-custom-length.js:
* Source/JavaScriptCore/builtins/ArrayIteratorPrototype.js:
(linkTimeConstant.arrayIteratorNextHelper):
* Source/JavaScriptCore/runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::isIteratorProtocolFastAndNonObservable):
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::installTypedArrayIteratorProtocolWatchpoint):
(JSC::JSGlobalObject::installTypedArrayPrototypeIteratorProtocolWatchpoint):
* Source/JavaScriptCore/runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::typedArrayPrototypeLengthAbsenceWatchpoint): Deleted.
* Source/JavaScriptCore/runtime/JSTypedArrayViewPrototype.cpp:
(JSC::JSTypedArrayViewPrototype::finishCreation):

Canonical link: https://commits.webkit.org/253153@main
  • Loading branch information
Constellation committed Aug 5, 2022
1 parent dc4fdf8 commit af1cc8c
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 41 deletions.
31 changes: 31 additions & 0 deletions JSTests/stress/detached-typed-array-iteration.js
@@ -0,0 +1,31 @@
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 array = new Uint8Array(42);
var count = 0;

shouldThrow(() => {
for (let v of array) {
++count;
$.detachArrayBuffer(array.buffer);
}
}, `TypeError: Underlying ArrayBuffer has been detached from the view`);

shouldBe(count, 1);
2 changes: 1 addition & 1 deletion JSTests/stress/typed-array-from-custom-length.js
Expand Up @@ -8,4 +8,4 @@ Reflect.defineProperty(array, 'length', {
value: 42
});
var result = Uint8Array.from(array);
shouldBe(result.length, 42);
shouldBe(result.length, 128); // Ignore "length"
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/builtins/ArrayIteratorPrototype.js
Expand Up @@ -50,7 +50,7 @@ function arrayIteratorNextHelper(array, kind)

var index = @getArrayIteratorInternalField(this, @arrayIteratorFieldIndex);
if (index !== -1) {
var length = array.length >>> 0;
var length = @isTypedArrayView(array) ? @typedArrayLength(array) : @toLength(array.length);
if (index < length) {
@putArrayIteratorInternalField(this, @arrayIteratorFieldIndex, index + 1);
done = false;
Expand Down
3 changes: 0 additions & 3 deletions Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
Expand Up @@ -371,9 +371,6 @@ bool JSArrayBufferView::isIteratorProtocolFastAndNonObservable()
if (getDirectOffset(vm, vm.propertyNames->iteratorSymbol) != invalidOffset)
return false;

if (getDirectOffset(vm, vm.propertyNames->length) != invalidOffset)
return false;

return true;
}

Expand Down
23 changes: 5 additions & 18 deletions Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Expand Up @@ -2617,39 +2617,26 @@ void JSGlobalObject::installTypedArrayIteratorProtocolWatchpoint(JSObject* base,
return ObjectPropertyCondition::absence(vm, this, base, propertyName.uid(), m_typedArrayProto.get(this));
};

ObjectPropertyCondition lengthCondition = absenceCondition(vm.propertyNames->length);
ObjectPropertyCondition iteratorCondition = absenceCondition(vm.propertyNames->iteratorSymbol);

if (!lengthCondition.isWatchable(PropertyCondition::EnsureWatchability) || !iteratorCondition.isWatchable(PropertyCondition::EnsureWatchability)) {
if (!iteratorCondition.isWatchable(PropertyCondition::EnsureWatchability)) {
typedArrayIteratorProtocolWatchpointSet(typedArrayType).invalidate(vm, StringFireDetail("Was not able to set up iterator protocol watchpoint."));
return;
}

RELEASE_ASSERT(!typedArrayIteratorProtocolWatchpointSet(typedArrayType).isBeingWatched());
typedArrayIteratorProtocolWatchpointSet(typedArrayType).touch(vm, "Set up iterator protocol watchpoint.");

typedArrayPrototypeLengthAbsenceWatchpoint(typedArrayType) = makeUnique<ObjectAdaptiveStructureWatchpoint>(this, lengthCondition, typedArrayIteratorProtocolWatchpointSet(typedArrayType));
typedArrayPrototypeLengthAbsenceWatchpoint(typedArrayType)->install(vm);
typedArrayPrototypeSymbolIteratorAbsenceWatchpoint(typedArrayType) = makeUnique<ObjectAdaptiveStructureWatchpoint>(this, iteratorCondition, typedArrayIteratorProtocolWatchpointSet(typedArrayType));
typedArrayPrototypeSymbolIteratorAbsenceWatchpoint(typedArrayType)->install(vm);
}

void JSGlobalObject::installTypedArrayPrototypeIteratorProtocolWatchpoint(JSTypedArrayViewPrototype* prototype, GetterSetter* lengthGetterSetter)
void JSGlobalObject::installTypedArrayPrototypeIteratorProtocolWatchpoint(JSTypedArrayViewPrototype* prototype)
{
VM& vm = this->vm();
{
ObjectPropertyCondition condition = setupAdaptiveWatchpoint(this, prototype, vm.propertyNames->iteratorSymbol);
m_typedArrayPrototypeSymbolIteratorWatchpoint = makeUnique<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>>(this, condition, m_typedArrayPrototypeIteratorProtocolWatchpointSet);
m_typedArrayPrototypeSymbolIteratorWatchpoint->install(vm);
}
{
PropertySlot slot(prototype, PropertySlot::InternalMethodType::VMInquiry, &vm);
prototype->getOwnPropertySlot(prototype, this, vm.propertyNames->length.impl(), slot);
prototype->structure()->startWatchingPropertyForReplacements(vm, slot.cachedOffset());
ObjectPropertyCondition speciesCondition = ObjectPropertyCondition::equivalence(vm, nullptr, prototype, vm.propertyNames->length.impl(), lengthGetterSetter);
m_typedArrayPrototypeLengthWatchpoint = makeUnique<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>>(this, speciesCondition, m_typedArrayPrototypeIteratorProtocolWatchpointSet);
m_typedArrayPrototypeLengthWatchpoint->install(vm);
}
ObjectPropertyCondition condition = setupAdaptiveWatchpoint(this, prototype, vm.propertyNames->iteratorSymbol);
m_typedArrayPrototypeSymbolIteratorWatchpoint = makeUnique<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>>(this, condition, m_typedArrayPrototypeIteratorProtocolWatchpointSet);
m_typedArrayPrototypeSymbolIteratorWatchpoint->install(vm);
}

void JSGlobalObject::installNumberPrototypeWatchpoint(NumberPrototype* numberPrototype)
Expand Down
18 changes: 1 addition & 17 deletions Source/JavaScriptCore/runtime/JSGlobalObject.h
Expand Up @@ -556,10 +556,8 @@ class JSGlobalObject : public JSSegmentedVariableObject {
std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayBufferPrototypeConstructorWatchpoints[2];
std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_typedArrayConstructorSpeciesWatchpoint;
std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_typedArrayPrototypeSymbolIteratorWatchpoint;
std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_typedArrayPrototypeLengthWatchpoint;
#define DECLARE_TYPED_ARRAY_TYPE_WATCHPOINT(name) \
std::unique_ptr<ObjectAdaptiveStructureWatchpoint> m_typedArray ## name ## ConstructorSpeciesAbsenceWatchpoint; \
std::unique_ptr<ObjectAdaptiveStructureWatchpoint> m_typedArray ## name ## PrototypeLengthAbsenceWatchpoint; \
std::unique_ptr<ObjectAdaptiveStructureWatchpoint> m_typedArray ## name ## PrototypeSymbolIteratorAbsenceWatchpoint; \
std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_typedArray ## name ## PrototypeConstructorWatchpoint;
FOR_EACH_TYPED_ARRAY_TYPE(DECLARE_TYPED_ARRAY_TYPE_WATCHPOINT)
Expand All @@ -579,20 +577,6 @@ class JSGlobalObject : public JSSegmentedVariableObject {
return m_typedArrayInt8ConstructorSpeciesAbsenceWatchpoint;
}

std::unique_ptr<ObjectAdaptiveStructureWatchpoint>& typedArrayPrototypeLengthAbsenceWatchpoint(TypedArrayType type)
{
switch (type) {
case NotTypedArray:
RELEASE_ASSERT_NOT_REACHED();
return m_typedArrayInt8PrototypeLengthAbsenceWatchpoint;
#define TYPED_ARRAY_TYPE_CASE(name) case Type ## name: return m_typedArray ## name ## PrototypeLengthAbsenceWatchpoint;
FOR_EACH_TYPED_ARRAY_TYPE(TYPED_ARRAY_TYPE_CASE)
#undef TYPED_ARRAY_TYPE_CASE
}
RELEASE_ASSERT_NOT_REACHED();
return m_typedArrayInt8PrototypeLengthAbsenceWatchpoint;
}

std::unique_ptr<ObjectAdaptiveStructureWatchpoint>& typedArrayPrototypeSymbolIteratorAbsenceWatchpoint(TypedArrayType type)
{
switch (type) {
Expand Down Expand Up @@ -1307,7 +1291,7 @@ class JSGlobalObject : public JSSegmentedVariableObject {
void tryInstallTypedArraySpeciesWatchpoint(TypedArrayType);
void installTypedArrayIteratorProtocolWatchpoint(JSObject* prototype, TypedArrayType);
void installTypedArrayConstructorSpeciesWatchpoint(JSTypedArrayViewConstructor*);
void installTypedArrayPrototypeIteratorProtocolWatchpoint(JSTypedArrayViewPrototype*, GetterSetter*);
void installTypedArrayPrototypeIteratorProtocolWatchpoint(JSTypedArrayViewPrototype*);

protected:
enum class HasSpeciesProperty : bool { Yes, No };
Expand Down
Expand Up @@ -565,7 +565,7 @@ void JSTypedArrayViewPrototype::finishCreation(VM& vm, JSGlobalObject* globalObj
putDirectWithoutTransition(vm, vm.propertyNames->builtinNames().valuesPublicName(), valuesFunction, static_cast<unsigned>(PropertyAttribute::DontEnum));
putDirectWithoutTransition(vm, vm.propertyNames->iteratorSymbol, valuesFunction, static_cast<unsigned>(PropertyAttribute::DontEnum));

globalObject->installTypedArrayPrototypeIteratorProtocolWatchpoint(this, lengthGetterSetter);
globalObject->installTypedArrayPrototypeIteratorProtocolWatchpoint(this);
}

JSTypedArrayViewPrototype* JSTypedArrayViewPrototype::create(
Expand Down

0 comments on commit af1cc8c

Please sign in to comment.